docs: correct claims about shipped platforms, SVG, and P/Invoke (Batch 1)#57
Conversation
…P/Invoke Batch 1 of the documentation-accuracy audit (#52). Fixes the "we shipped it but the docs say we didn't" cluster: - F1 product.md / BRIEF.md: rendering is native on Apple, Linux, Android (Kotlin Canvas), Windows/.NET (SkiaSharp), WebAssembly, and Flutter over the platform-free RenderScene/SceneWire contract — not Apple(+Linux)-only. Drops the "planned / not shipped Android renderer" scope notes. (Bumps the co-located product.md Status v1.4.0 -> v2.2.0, since it shares the sentence.) - F2 CONTRIBUTING.md / BRIEF.md: the SVG backend shipped (MermaidRenderer.svg / SVGRenderer) — removed from "most-wanted"/roadmap and the "Don't claim SVG output" site instruction. - F3 docs/notes/windows.md / windows/README.md: the P/Invoke bridge is implemented, tested (PInvokeTests), and CI-gated on windows-latest — moved to "Done + verified"; the only open Windows item is the device-font measure callback. Also fixes the diagram DLL label mermaidkit.dll -> MermaidKitCShared.dll (F13). - F4 docs/notes/android.md: fromMaterial() -> fromMaterial(MaterialTheme.colorScheme) (no zero-arg overload exists — the snippet would not compile). - F5 tools/pi-canvas/README.md: qualify the zoom-reraster crispness claim — it holds on the Apple/CoreGraphics path; on Linux/Silica silicaRaster ignores targetWidth, so zoomed cards are repositioned, not re-rasterized larger. Closes #52. Advances #53 (product.md version) and #56 (F13). Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_014CJYAapHgDn2PWGNABnFtv
WalkthroughDocumentation updates correct claims about supported rendering platforms, SVG and RenderScene scope, Windows P/Invoke status, Android Compose theming, InfiniteCanvas zoom behavior, and current contribution priorities. ChangesPlatform and rendering claims
Windows bridge status
Platform-specific usage guidance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/website/BRIEF.md`:
- Around line 113-116: Update the platform/toolchain statement in BRIEF.md to
use the project-wide minimums of Swift 6.2 and Xcode 26 instead of Swift 6 and
Xcode 16, preserving the surrounding platform and OS-floor wording.
In `@tools/pi-canvas/README.md`:
- Around line 15-20: Update the InfiniteCanvas description so it states that
each diagram is rendered once per zoom level and cached, while preserving the
existing explanation of Apple/CoreGraphics re-rasterization and Linux/Silica
behavior.
In `@windows/README.md`:
- Around line 50-54: Add the unfinished per-architecture NuGet packaging of
MermaidKitCShared.dll to the “Not yet here (next slices)” list in
windows/README.md, matching the corresponding item in docs/notes/windows.md.
Keep the existing device-font measure callback item unchanged.
🪄 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
Run ID: c909c818-c6f9-4b76-b9c7-0c3175359545
📒 Files selected for processing (7)
CONTRIBUTING.mddocs/notes/android.mddocs/notes/windows.mddocs/product.mddocs/website/BRIEF.mdtools/pi-canvas/README.mdwindows/README.md
💤 Files with no reviewable changes (1)
- CONTRIBUTING.md
| - Platforms: renders natively on Apple (macOS 14+, iOS 17+, visionOS 1+), | ||
| Linux, Android, Windows/.NET, WebAssembly, and Flutter, from one Swift | ||
| layout core (Swift 6 language mode, Xcode 16+ to build). Lower Apple OS | ||
| floors are the top of the public roadmap. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== docs/product.md (relevant lines) ==\n'
sed -n '1,80p' docs/product.md | cat -n
printf '\n== docs/website/BRIEF.md (relevant lines) ==\n'
sed -n '100,130p' docs/website/BRIEF.md | cat -n
printf '\n== Search for Xcode/toolchain floors across docs ==\n'
rg -n "Xcode 1[0-9]\+|Xcode 2[0-9]\+|swift-tools-version|swift tools version|tools version" docs -SRepository: 2389-research/MermaidKit
Length of output: 11341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Search for Xcode 16+ / 26 mentions outside the two docs ==\n'
rg -n "Xcode 16\+|Xcode 26|swift-tools-version: 6\.2|Swift 6\.2" -S .
printf '\n== Nearby context in README.md if present ==\n'
if [ -f README.md ]; then
rg -n "Xcode 16\+|Xcode 26|swift-tools-version: 6\.2|Swift 6\.2" README.md -n -S
fiRepository: 2389-research/MermaidKit
Length of output: 2692
Keep the toolchain floor aligned with the rest of the docs. docs/website/BRIEF.md still says Xcode 16+ / Swift 6, but docs/product.md, README.md, CONTRIBUTING.md, and the changelog all state the current floor is Swift 6.2 / Xcode 26. Update this line to match the project-wide requirement.
🤖 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 `@docs/website/BRIEF.md` around lines 113 - 116, Update the platform/toolchain
statement in BRIEF.md to use the project-wide minimums of Swift 6.2 and Xcode 26
instead of Swift 6 and Xcode 16, preserving the surrounding platform and
OS-floor wording.
| - **`InfiniteCanvas`** — diagrams placed at positions in an unbounded virtual | ||
| space. Each is rendered once to an RGBA raster (cached; zoom re-rasterizes for | ||
| crispness rather than scaling a bitmap). Each frame culls to the viewport and | ||
| blits only the visible cards. | ||
| space. Each is rendered once to an RGBA raster (cached). On the Apple/CoreGraphics | ||
| path, zooming re-rasterizes for crispness rather than scaling a bitmap; on the | ||
| Linux/Silica path `silicaRaster` returns the natural render size and ignores the | ||
| zoom-scaled width, so zoomed cards are repositioned but not re-rasterized larger. | ||
| Each frame culls to the viewport and blits only the visible cards. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify that caching is per zoom level.
“Each is rendered once” contradicts the documented Apple behavior, where zooming re-rasterizes and InfiniteCanvas.swift caches rasters by diagram index and zoom. Say that each diagram is rendered once per zoom level and cached.
Suggested wording
-- **`InfiniteCanvas`** — diagrams placed at positions in an unbounded virtual
- space. Each is rendered once to an RGBA raster (cached). On the Apple/CoreGraphics
+- **`InfiniteCanvas`** — diagrams placed at positions in an unbounded virtual
+ space. Each is rendered once per zoom level to an RGBA raster (cached). On the Apple/CoreGraphics📝 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.
| - **`InfiniteCanvas`** — diagrams placed at positions in an unbounded virtual | |
| space. Each is rendered once to an RGBA raster (cached; zoom re-rasterizes for | |
| crispness rather than scaling a bitmap). Each frame culls to the viewport and | |
| blits only the visible cards. | |
| space. Each is rendered once to an RGBA raster (cached). On the Apple/CoreGraphics | |
| path, zooming re-rasterizes for crispness rather than scaling a bitmap; on the | |
| Linux/Silica path `silicaRaster` returns the natural render size and ignores the | |
| zoom-scaled width, so zoomed cards are repositioned but not re-rasterized larger. | |
| Each frame culls to the viewport and blits only the visible cards. | |
| - **`InfiniteCanvas`** — diagrams placed at positions in an unbounded virtual | |
| space. Each is rendered once per zoom level to an RGBA raster (cached). On the Apple/CoreGraphics | |
| path, zooming re-rasterizes for crispness rather than scaling a bitmap; on the | |
| Linux/Silica path `silicaRaster` returns the natural render size and ignores the | |
| zoom-scaled width, so zoomed cards are repositioned but not re-rasterized larger. | |
| Each frame culls to the viewport and blits only the visible cards. |
🤖 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 `@tools/pi-canvas/README.md` around lines 15 - 20, Update the InfiniteCanvas
description so it states that each diagram is rendered once per zoom level and
cached, while preserving the existing explanation of Apple/CoreGraphics
re-rasterization and Linux/Silica behavior.
| ## Not yet here (next slices) | ||
|
|
||
| - **The P/Invoke bridge** — this library renders a `SceneWire` you hand it; wiring | ||
| `MermaidNative` → `[DllImport]` → `mmk_scene_json` in a Swift-built Windows DLL | ||
| (so the app passes a Mermaid *source string* + a measure callback) is the next | ||
| step, mirroring Android's JNI seam. | ||
| - **Device-font measure callback** — the first P/Invoke slice passes none, so native | ||
| layout uses a coarse glyph-box metric; threading a measure callback through the ABI | ||
| lets layout measure with the face that draws. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the remaining-work list consistent with docs/notes/windows.md.
The Windows notes also identify bundling MermaidKitCShared.dll in a per-architecture NuGet package as unfinished. Add that item here, or explicitly state that this list is non-exhaustive, so readers do not mistake the documented next slices for the complete roadmap.
🤖 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 `@windows/README.md` around lines 50 - 54, Add the unfinished per-architecture
NuGet packaging of MermaidKitCShared.dll to the “Not yet here (next slices)”
list in windows/README.md, matching the corresponding item in
docs/notes/windows.md. Keep the existing device-font measure callback item
unchanged.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce273fd1e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| top of the public roadmap. | ||
| - Platforms: renders natively on Apple (macOS 14+, iOS 17+, visionOS 1+), | ||
| Linux, Android, Windows/.NET, WebAssembly, and Flutter, from one Swift | ||
| layout core (Swift 6 language mode, Xcode 16+ to build). Lower Apple OS |
There was a problem hiding this comment.
The refreshed platform paragraph now describes the current multi-platform v2.x state, but it still says Xcode 16+ is sufficient. The package manifest requires SwiftPM 6.2, and the updated contributing guide says the floor is Swift 6.2 / Xcode 26, so site copy generated from this brief would tell developers on Xcode 16 to use a toolchain that cannot parse/build the package. Update this floor alongside the new platform claims.
Useful? React with 👍 / 👎.
1 participant
Batch 1 of the doc-accuracy audit — the 5 high-severity "shipped but documented as unbuilt" fixes. Closes #52.
Every claim verified against code before editing:
product.md/BRIEF.mdsaid rendering is Apple(+Linux)-only and the Android renderer is "planned, not shipped." Reality: native renderers on Apple, Linux, Android (KotlinCanvas), Windows/.NET (SkiaSharp), WASM, and Flutter overRenderScene/SceneWire(README:12, CHANGELOG 2.0.0).MermaidRenderer.svgoverSVGRenderer(MermaidRenderer+SVG.swift:39). Removed from roadmap + dropped "Don't claim SVG output."MermaidNative.cs[DllImport("MermaidKitCShared")]), tested (PInvokeTests.cs), CI-gated onwindows-latest. Only the device-font measure callback remains. Also fixed the diagram DLL name (F13).MermaidTheme.fromMaterial()won't compile (no zero-arg overload) →fromMaterial(MaterialTheme.colorScheme).silicaRasterignorestargetWidth); qualified per-backend.Also advances #53 (product.md
v1.4.0→v2.2.0, co-located in the same sentence) and #56 (F13).Docs-only; no code changed.
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit