replace mermaid chat rendering with vizlayer#168
replace mermaid chat rendering with vizlayer#168FrankChen021 wants to merge 3 commits intomasterfrom
Conversation
Route structured diagrams through the published Vizlayer renderer, remove the legacy Mermaid-specific markdown component, and surface loaded skill resource inputs in the chat tool UI.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR migrates chat diagram rendering away from the legacy Mermaid-only path to the published Vizlayer renderer, while also improving skill tool transparency by displaying loaded skill inputs in the chat UI.
Changes:
- Route
vizlayercode fences through a newMessageMarkdownVizlayerrenderer and stop special-casing Mermaid fences. - Replace the Mermaid markdown component and remove
markdown-it-mermaid; add@vizlayer/react. - Add Vizlayer skill reference docs and regression tests for updated chat message/tool flows.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/chat/message/message-tool-skill.tsx | Enhances the skill tool UI to display requested skill names/resource paths and uses them for the collapsible header. |
| src/components/chat/message/message-tool-skill.test.tsx | Adds tests validating skill/resource inputs show up in the tool header/body. |
| src/components/chat/message/message-markdown.tsx | Routes language-vizlayer fences to the new Vizlayer renderer; Mermaid is no longer specially rendered. |
| src/components/chat/message/message-markdown.test.tsx | Adds tests ensuring Mermaid fences render as plain code and vizlayer fences are routed to Vizlayer renderer. |
| src/components/chat/message/message-markdown-vizlayer.tsx | Introduces the Vizlayer renderer component with copy/error states and theme selection. |
| src/components/chat/message/message-markdown-vizlayer.test.tsx | Adds component-level tests around parsing/copy/error handling behavior (with mocked Vizlayer APIs). |
| src/components/chat/message/message-markdown-mermaid.tsx | Removes the legacy Mermaid-specific renderer component. |
| resources/skills/vizlayer/SKILL.md | Adds the Vizlayer skill manual for authoring unified Vizlayer payloads. |
| resources/skills/vizlayer/reference/flowchart.md | Adds a flowchart payload reference for the Vizlayer skill. |
| resources/skills/vizlayer/reference/sequence-diagram.md | Adds a sequence diagram payload reference for the Vizlayer skill. |
| resources/skills/vizlayer/reference/class-diagram.md | Adds a class diagram payload reference for the Vizlayer skill. |
| package.json | Adds @vizlayer/react and removes markdown-it-mermaid. |
| package-lock.json | Updates lockfile for the new Vizlayer dependency graph and removal of markdown-it-mermaid. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return input.resources.flatMap((resource) => | ||
| resource.paths | ||
| .map((path) => path.trim()) | ||
| .filter(Boolean) | ||
| .map((path) => `${resource.skill} | ${path}`) | ||
| ); |
There was a problem hiding this comment.
getRequestedItems assumes every resources[] entry has a paths array of strings and a skill string. Since toolPart.input is unknown and is cast to SkillInput, a malformed/corrupted message could throw at resource.paths.map(...) and break chat rendering. Consider validating each resource item (e.g., Array.isArray(resource.paths) + typeof resource.skill === "string") and skipping invalid entries instead of throwing.
| return input.resources.flatMap((resource) => | |
| resource.paths | |
| .map((path) => path.trim()) | |
| .filter(Boolean) | |
| .map((path) => `${resource.skill} | ${path}`) | |
| ); | |
| return input.resources.flatMap((resource) => { | |
| if (!resource || typeof resource !== "object") { | |
| return []; | |
| } | |
| const paths = (resource as { paths?: unknown }).paths; | |
| const skill = (resource as { skill?: unknown }).skill; | |
| if (!Array.isArray(paths) || typeof skill !== "string") { | |
| return []; | |
| } | |
| return paths | |
| .filter((path): path is string => typeof path === "string") | |
| .map((path) => path.trim()) | |
| .filter(Boolean) | |
| .map((path) => `${skill} | ${path}`); | |
| }); |
| {requestedItems.map((item) => ( | ||
| <div key={item}>{item}</div> |
There was a problem hiding this comment.
requestedItems.map((item) => <div key={item}>...) uses the display string as the React key. If the input contains duplicates (same skill name repeated or same resource path listed twice), this will produce duplicate keys and React warnings; prefer a key that is guaranteed unique (e.g. include the index or use a structured {skill,path} key).
| {requestedItems.map((item) => ( | |
| <div key={item}>{item}</div> | |
| {requestedItems.map((item, index) => ( | |
| <div key={index}>{item}</div> |
| const isDark = useMemo(() => { | ||
| if (typeof window !== "undefined") { | ||
| return window.document.documentElement.classList.contains("dark"); | ||
| } | ||
|
|
||
| return theme === "dark"; | ||
| }, [theme]); |
There was a problem hiding this comment.
isDark is memoized and only recomputed when theme changes, but it reads document.documentElement.classList. If the root dark class changes without a theme state change (e.g. system theme transitions or other DOM-driven toggles), diagrams can get stuck with the wrong theme. Consider switching to the existing useIsDarkTheme hook (which observes class changes) or adding a MutationObserver here so the diagram theme updates when the DOM class changes.
| it("still parses complete payloads with trailing whitespace after the closing brace", () => { | ||
| toChartSpecSpy.mockReturnValue("flowchart TD\n a --> b"); | ||
| parseVizlayerSpecSpy.mockReturnValue({ | ||
| ok: true, | ||
| spec: { | ||
| kind: "flowchart", | ||
| document: { | ||
| direction: "TD", | ||
| nodes: [ | ||
| { id: "a", label: "A" }, | ||
| { id: "b", label: "B" }, | ||
| ], | ||
| edges: [{ from: "a", to: "b" }], | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| act(() => { | ||
| root.render( | ||
| <MessageMarkdownVizlayer | ||
| spec={ | ||
| '{"kind":"flowchart","document":{"direction":"TD","nodes":[{"id":"a","label":"A"},{"id":"b","label":"B"}],"edges":[{"from":"a","to":"b"}]}}\n ' | ||
| } | ||
| /> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
The tests in this file mock VizlayerSpecParser.parseVizlayerSpec, so cases like “parses complete payloads with trailing whitespace” aren’t actually verifying parsing behavior—only that the component calls the mocked parser. Consider renaming those test descriptions to reflect the mocked interaction, or add at least one integration-style test that uses the real parser to cover whitespace/kind validation behavior end-to-end.
Relocate the ClickHouse skill source into external submodules, add Vizlayer as a built local dependency, and sync generated skill assets into resources during install and build so app, Docker, and CI flows stay aligned.
Harden skill input rendering against malformed payloads, make Vizlayer theme tracking follow DOM class changes, and remove the obsolete user_actions markdown flow from the chat pipeline.
Summary
vizlayerfences throughMessageMarkdownVizlayermarkdown-it-mermaiddependencyTest plan
npm run formatnpx vitest run "src/components/chat/message/message-markdown.test.tsx" "src/components/chat/message/message-markdown-vizlayer.test.tsx" "src/components/chat/message/message-tool-skill.test.tsx"npm run typechecknpm run lint -- "src/components/chat/message/message-markdown.tsx" "src/components/chat/message/message-markdown.test.tsx" "src/components/chat/message/message-markdown-vizlayer.tsx" "src/components/chat/message/message-markdown-vizlayer.test.tsx" "src/components/chat/message/message-tool-skill.tsx" "src/components/chat/message/message-tool-skill.test.tsx"npm run build(blocked in this worktree becauseexternal/number-flow/packages/number-flow/package.jsonis missing, so the dependency prebuild step fails before the app build runs)