Document architecture, Skippy diagrams, and model packages - #974
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds Mermaid rendering to documentation pages and introduces architecture and model-package documentation, including navigation, request-flow diagrams, split-serving guidance, manifest rules, validation requirements, and compatibility conventions. ChangesArchitecture documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review. |
![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: 6
🧹 Nitpick comments (1)
website/src/assets/mermaid.js (1)
1-49: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd error handling for CDN import and rendering failures.
If the CDN import fails or
mermaid.runthrows, the error becomes an unhandled rejection. The fallback (raw code visible) is acceptable UX, but wrapping intry/catchprevents console noise and makes failures observable.🛡️ Proposed fix
async function renderMermaid() { const blocks = document.querySelectorAll("pre > code.language-mermaid"); if (!blocks.length) { return; } - const { default: mermaid } = await import( - "https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.esm.min.mjs" - ); - - mermaid.initialize({ - startOnLoad: false, - securityLevel: "strict", - theme: "dark", - flowchart: { - curve: "basis", - htmlLabels: true, - }, - sequence: { - useMaxWidth: true, - }, - }); - - const nodes = []; - - blocks.forEach((code) => { - const container = document.createElement("div"); - const pre = code.parentElement; - const frame = pre?.parentElement?.classList.contains("code-copy-frame") - ? pre.parentElement - : pre; - - container.className = "mermaid"; - container.setAttribute("role", "img"); - container.setAttribute("aria-label", "Diagram"); - container.textContent = code.textContent; - frame?.replaceWith(container); - nodes.push(container); - }); - - mermaid.run({ nodes }); + try { + const { default: mermaid } = await import( + "https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.esm.min.mjs" + ); + + mermaid.initialize({ + startOnLoad: false, + securityLevel: "strict", + theme: "dark", + flowchart: { + curve: "basis", + htmlLabels: true, + }, + sequence: { + useMaxWidth: true, + }, + }); + + const nodes = []; + + blocks.forEach((code) => { + const container = document.createElement("div"); + const pre = code.parentElement; + const frame = pre?.parentElement?.classList.contains("code-copy-frame") + ? pre.parentElement + : pre; + + container.className = "mermaid"; + container.setAttribute("role", "img"); + container.setAttribute("aria-label", "Diagram"); + container.textContent = code.textContent; + frame?.replaceWith(container); + nodes.push(container); + }); + + await mermaid.run({ nodes }); + } catch (err) { + console.error("Mermaid rendering failed:", err); + } }🤖 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 `@website/src/assets/mermaid.js` around lines 1 - 49, Update renderMermaid to catch failures from the dynamic Mermaid CDN import and the subsequent mermaid.run call, preventing unhandled rejections while preserving the raw-code fallback. Make the caught failures observable through appropriate error logging, and keep the existing rendering flow unchanged when no error occurs.
🤖 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 `@website/src/assets/mermaid.js`:
- Around line 27-40: Update the Mermaid container labeling in the
block-processing loop to avoid assigning every diagram the inaccurate
“Architecture diagram” label. Derive an accessible label from each diagram’s
content when possible, or use a generic label that accurately applies to all
Mermaid diagrams, while preserving the existing container creation and
replacement behavior.
- Line 42: Update the async flow containing mermaid.run({ nodes }) to await the
returned Promise, keeping it within the surrounding try/catch so rendering
errors are properly propagated and handled.
- Around line 8-23: Update the mermaid.initialize configuration in
website/src/assets/mermaid.js to use an HTML-permitting security level so the
existing htmlLabels and <br/> labels render correctly, while preserving the
other Mermaid options.
In `@website/src/assets/site.css`:
- Around line 4506-4521: Update the .docs-body .doc .mermaid background
declaration to use the existing design-system CSS custom property matching this
dark surface color, such as --bg-1 or --surface-1. If no suitable property
exists, define the required custom property with the root variables and
reference it here, removing the hardcoded `#07090d` value.
In `@website/src/docs/pages/model-package-spec.md`:
- Line 130: Update the activation_width entry in the model package specification
table to mark it as Required instead of Recommended, matching the manifest
validation contract enforced by the model package implementation.
- Around line 236-239: Clarify the checksum-verification wording in the
artifact-integrity section: require each downloaded artifact to be checked
against its corresponding manifest entry’s artifact_bytes and sha256 before
installation. Remove or replace the ambiguous “manifest size” requirement, while
preserving the requirement to atomically install files from fresh partial files.
---
Nitpick comments:
In `@website/src/assets/mermaid.js`:
- Around line 1-49: Update renderMermaid to catch failures from the dynamic
Mermaid CDN import and the subsequent mermaid.run call, preventing unhandled
rejections while preserving the raw-code fallback. Make the caught failures
observable through appropriate error logging, and keep the existing rendering
flow unchanged when no error occurs.
🪄 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: 8d43ae6e-4899-4270-b82e-3a8c70c1e95a
📒 Files selected for processing (9)
website/.eleventy.jswebsite/src/_data/docs.jswebsite/src/_includes/docs-base.njkwebsite/src/assets/mermaid.jswebsite/src/assets/site.csswebsite/src/docs/pages/architecture.mdwebsite/src/docs/pages/contributing-layer-packages.mdwebsite/src/docs/pages/model-package-spec.mdwebsite/src/docs/pages/running-large-models.md
Comment on lines
+4506
to
+4521
| .docs-body .doc .mermaid { | ||
| margin: 24px 0; | ||
| overflow-x: auto; | ||
| padding: 16px; | ||
| border: 1px solid var(--line-2); | ||
| border-radius: 8px; | ||
| background: #07090d; | ||
| text-align: center; | ||
| } | ||
|
|
||
| .docs-body .doc .mermaid svg { | ||
| display: inline-block; | ||
| max-width: 100%; | ||
| height: auto; | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a CSS custom property for the mermaid container background color.
The border correctly uses var(--line-2), but background: #07090d`` is a hardcoded hex value. As per coding guidelines, stylesheets should use CSS custom properties for theming and follow the dark-first color palette defined in design.json for all colors.
🎨 Proposed fix
.docs-body .doc .mermaid {
margin: 24px 0;
overflow-x: auto;
padding: 16px;
border: 1px solid var(--line-2);
border-radius: 8px;
- background: `#07090d`;
+ background: var(--bg-1, `#07090d`);
text-align: center;
}If a matching custom property (e.g., --bg-1 or --surface-1) already exists in the design system, use it directly. Otherwise, define one in the root variables and reference it here.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .docs-body .doc .mermaid { | |
| margin: 24px 0; | |
| overflow-x: auto; | |
| padding: 16px; | |
| border: 1px solid var(--line-2); | |
| border-radius: 8px; | |
| background: #07090d; | |
| text-align: center; | |
| } | |
| .docs-body .doc .mermaid svg { | |
| display: inline-block; | |
| max-width: 100%; | |
| height: auto; | |
| } | |
| .docs-body .doc .mermaid { | |
| margin: 24px 0; | |
| overflow-x: auto; | |
| padding: 16px; | |
| border: 1px solid var(--line-2); | |
| border-radius: 8px; | |
| background: var(--bg-1, `#07090d`); | |
| text-align: center; | |
| } | |
| .docs-body .doc .mermaid svg { | |
| display: inline-block; | |
| max-width: 100%; | |
| height: auto; | |
| } |
🤖 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 `@website/src/assets/site.css` around lines 4506 - 4521, Update the .docs-body
.doc .mermaid background declaration to use the existing design-system CSS
custom property matching this dark surface color, such as --bg-1 or --surface-1.
If no suitable property exists, define the required custom property with the
root variables and reference it here, removing the hardcoded `#07090d` value.
Source: Coding guidelines
ndizazzo
approved these changes
Jul 13, 2026
ndizazzo
left a comment
ndizazzo
left a comment
Collaborator
There was a problem hiding this comment.
Minor comments for improvements
| ```mermaid | ||
| flowchart TD | ||
| App["Application<br/>OpenAI client · SDK · console · plugin"] | ||
| APIs["Node APIs<br/>9337 /v1<br/>3131 /api"] |
Collaborator
There was a problem hiding this comment.
Might be good to have a blurb for the owned node-control API too (config + commands for attested hosts).
|
|
||
| ### Routing and election | ||
|
|
||
| Every node exposes the same OpenAI-facing shape. A request is routed by model identity rather than by a user selecting a machine. Per-model election decides which node or stage-0 target is authoritative, while passive clients receive a smaller route view instead of full mesh gossip. |
Collaborator
There was a problem hiding this comment.
Every node -> Every host / worker node
| | FFI and language packages | `crates/mesh-llm-ffi/`, `crates/mesh-llm-nodejs/`, `sdk/` | | ||
| | Skippy runtime and stage serving | `crates/skippy-*` and `crates/mesh-llm-embedded-runtime/` | | ||
| | Protocol definitions | `crates/mesh-llm-protocol/`, `crates/skippy-protocol/`, `proto/` | | ||
|
|
Collaborator
There was a problem hiding this comment.
Can add a line for the React Mesh LLM console too
| @@ -0,0 +1,284 @@ | |||
| --- | |||
Collaborator
There was a problem hiding this comment.
This spec is hard line-wrapped and our other docs allow soft line wrapping from markdown renderers... we probably want to remove the limit and the renderer handle wrapping
|
|
||
| The [architecture hub](/docs/pages/architecture/) explains how Mesh routes requests into Skippy. See the [model package specification](/docs/pages/model-package-spec/) for the manifest schema, artifact checksums, and stage-selection rules. For package publishing and validation, see [Layer package repositories](https://github.com/Mesh-LLM/mesh-llm/blob/main/docs/LAYER_PACKAGE_REPOS.md). | ||
|
|
||
| If you are just trying Mesh for the first time, do not start here. Run the [Quickstart](/docs/pages/quickstart/) first. |
Merged
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
2 participants
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
model-package.jsonschema specification covering layout, fields, artifact integrity, stage selection, generation defaults, validation, and compatibility.Why
The website had no single architecture map or public contract for model-package repositories. These docs make the mesh/Skippy execution path and package format easier to understand and implement against.
Validation
just website-buildgit diff --checknode --check website/src/assets/mermaid.jsSummary by CodeRabbit
model-package.jsoncontract.