feat(maestro): enrich lsp markdown docs#2739
Merged
Merged
Conversation
vize
@vizejs/fresco
@vizejs/musea-mcp-server
oxlint-plugin-vize
@vizejs/rspack-plugin
@vizejs/unplugin
@vizejs/vite-plugin
@vizejs/vite-plugin-musea
@vizejs/musea-nuxt
@vizejs/nuxt
commit: |
📝 WalkthroughWalkthroughThis PR adds a ChangesMarkdown builder and its adoption
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant CompletionService
participant component_docs
participant Markdown
participant CompletionItem
CompletionService->>component_docs: prop_documentation/slot_documentation
component_docs->>Markdown: build title/meta/code/example sections
Markdown-->>component_docs: Documentation
component_docs-->>CompletionService: Documentation
CompletionService->>CompletionItem: set documentation
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
PR BenchmarkBase:
Raw run timesCompile SFC
Lint
Format
Type check (1T)
Type check (max)
|
Contributor
Detailed Test ReportCommit: Area Summary
Test InventoryTotal tracked cases: 8644 across 1221 files.
Files
Comment truncated at 64000 characters. Open the workflow run for the full job log. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@crates/vize_maestro/src/ide/completion/items.rs`:
- Around line 342-347: The markdown_doc test helper is duplicated across
multiple test files, so extract the shared logic into a common test utility
module and update callers to use it. Move the helper currently defined in
markdown_doc so native_tests.rs, template_event_tests.rs, and
component_props_tests.rs all reference the same function, keeping the
Documentation matching behavior identical in one place to prevent drift.
- Around line 317-348: The Markdown documentation assertions only cover
directive_item and attr_item, so the other migrated completion constructors are
not verified. Extend the tests in tests to add coverage for vize_directive_item,
component_item, api_item, macro_item, and css_item, and assert their
documentation content through markdown_doc so each constructor’s docs are
checked for the expected Markdown formatting and links.
In `@crates/vize_maestro/src/ide/completion/template/component_docs.rs`:
- Around line 1-2: The clippy suppression in component_docs is too broad because
it is applied at module scope, which will hide future disallowed-macros uses in
this file. Remove the file-level `#![allow(clippy::disallowed_macros)]` and
scope the allowance only to the specific helper functions in `component_docs`
that need it, following the same per-function pattern used in `markup.rs`.
In `@crates/vize_maestro/src/ide/hover/builder.rs`:
- Around line 39-47: The example/docs formatting is duplicated by manually
rebuilding the same heading/body pattern instead of reusing the existing section
helper. Update HoverBuilder::example and the matching docs method to delegate
through self.section(...) (or a shared helper) so the "**{heading}**\n\n{body}"
logic lives in one place, and remove the now-unnecessary allow on those methods.
In `@crates/vize_maestro/src/ide/markup.rs`:
- Around line 87-98: The bare tabstop handling in snippet_for_docs only consumes
a single digit after $, so multi-digit placeholders like $10 are rendered
incorrectly. Update the cursor advance in the snippet parsing loop in markup.rs
to consume the full consecutive digit run for the bare $N case, matching the
braced tabstop behavior, and keep the existing collapse-to-... logic for any
nonzero tabstop index.
In `@tests/tooling/lsp-authoring-core.test.ts`:
- Around line 93-95: The hover test is asserting markdown wrappers instead of
the real behavior, making it brittle to cosmetic wording changes. Update the
assertions around clickHoverText in the lsp-authoring-core test to check for the
actual example/handler snippet content that should appear in the hover, rather
than the **Example** label or vue fence markers. Keep the checks focused on
observable hover text from the Vue event-handling example so failures reflect
real regressions, not formatting churn.
In `@tests/tooling/lsp-editor-production-gates.test.ts`:
- Around line 165-166: The assertions in the gating tests are too
formatting-specific; update the checks in the affected test cases to verify the
actual example content or observable disabled/enabled behavior instead of
markdown markers like bold headings or fenced code blocks. Use the existing test
block in lsp-editor-production-gates.test.ts to locate the impacted assertions
and replace them with focused expectations that validate the semantic output
from the example content.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e97587f5-fc59-47fc-ae6e-f5ce910f31fd
📒 Files selected for processing (16)
crates/vize_maestro/src/ide.rscrates/vize_maestro/src/ide/completion/component_props_tests.rscrates/vize_maestro/src/ide/completion/items.rscrates/vize_maestro/src/ide/completion/template.rscrates/vize_maestro/src/ide/completion/template/component_docs.rscrates/vize_maestro/src/ide/completion/template/component_meta.rscrates/vize_maestro/src/ide/completion/template/directives.rscrates/vize_maestro/src/ide/completion/template/native_tests.rscrates/vize_maestro/src/ide/completion/template_event_tests.rscrates/vize_maestro/src/ide/hover/builder.rscrates/vize_maestro/src/ide/hover/component_prop.rscrates/vize_maestro/src/ide/hover/html.rscrates/vize_maestro/src/ide/hover/template.rscrates/vize_maestro/src/ide/markup.rstests/tooling/lsp-authoring-core.test.tstests/tooling/lsp-editor-production-gates.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: playground-test
- GitHub Check: test-js-packages
- GitHub Check: criterion-ab
- GitHub Check: tool-benchmark
🧰 Additional context used
📓 Path-based instructions (2)
tests/tooling/**
⚙️ CodeRabbit configuration file
tests/tooling/**: Ensure workflow and release tests assert observable behavior rather than incidental formatting. Prefer focused assertions that keep PRs small and failures easy to diagnose.
Files:
tests/tooling/lsp-authoring-core.test.tstests/tooling/lsp-editor-production-gates.test.ts
crates/**
⚙️ CodeRabbit configuration file
crates/**: Focus on parser/compiler correctness, source locations, UTF-8/UTF-16 offset handling, and panic-free LSP behavior. Flag changes that only work for small fixtures but break real Vue/Nuxt projects.
Files:
crates/vize_maestro/src/ide/completion/template_event_tests.rscrates/vize_maestro/src/ide/completion/template/native_tests.rscrates/vize_maestro/src/ide.rscrates/vize_maestro/src/ide/completion/template.rscrates/vize_maestro/src/ide/completion/template/component_docs.rscrates/vize_maestro/src/ide/hover/html.rscrates/vize_maestro/src/ide/completion/component_props_tests.rscrates/vize_maestro/src/ide/hover/template.rscrates/vize_maestro/src/ide/completion/template/directives.rscrates/vize_maestro/src/ide/markup.rscrates/vize_maestro/src/ide/hover/component_prop.rscrates/vize_maestro/src/ide/completion/template/component_meta.rscrates/vize_maestro/src/ide/completion/items.rscrates/vize_maestro/src/ide/hover/builder.rs
🔇 Additional comments (18)
crates/vize_maestro/src/ide/hover/builder.rs (1)
1-3: LGTM!Also applies to: 81-84, 93-107
crates/vize_maestro/src/ide/hover/component_prop.rs (1)
31-40: LGTM!Also applies to: 74-78, 154-155
crates/vize_maestro/src/ide/hover/html.rs (1)
26-26: LGTM!Also applies to: 44-45, 75-76, 137-148, 177-178, 201-202
crates/vize_maestro/src/ide/hover/template.rs (1)
137-155: LGTM!Also applies to: 161-178, 362-368
crates/vize_maestro/src/ide.rs (1)
32-32: LGTM!Also applies to: 72-73
crates/vize_maestro/src/ide/markup.rs (1)
1-150: LGTM!crates/vize_maestro/src/ide/completion/items.rs (1)
95-315: LGTM!crates/vize_maestro/src/ide/completion/template/native_tests.rs (1)
3-3: LGTM!Also applies to: 24-49, 94-118
crates/vize_maestro/src/ide/completion/template_event_tests.rs (1)
9-11: LGTM!Also applies to: 30-33, 82-88
crates/vize_maestro/src/ide/completion/template.rs (1)
13-13: LGTM!crates/vize_maestro/src/ide/completion/template/component_docs.rs (3)
8-39: LGTM! Parameter order matches the call site incomponent_meta.rs, and the conditionalDefaultsection is composed correctly.
41-73: LGTM!
75-91: LGTM!crates/vize_maestro/src/ide/completion/template/component_meta.rs (2)
7-15: LGTM! Theinsert_name.clone()avoids the earlier move so it can still be borrowed forcomponent_docs::prop_documentation, and the call's argument order matches the helper's signature.Also applies to: 319-337, 349-349
367-367: LGTM! Matching ondestructure.as_ref()correctly preserves ownership ofdestructureso it can be reused via.as_deref()forslot_documentation.Also applies to: 400-401
crates/vize_maestro/src/ide/completion/template/directives.rs (2)
4-9: LGTM!Also applies to: 67-84
126-150: LGTM!crates/vize_maestro/src/ide/completion/component_props_tests.rs (1)
3-3: LGTM! The test now exercises the completion item'sdocumentationfield directly rather than only labels, andmarkdown_doccorrectly unwraps bothDocumentationvariants (confirmed against thelsp_types::Documentation/MarkupContentAPI).Also applies to: 44-60, 218-224
Comment on lines
+317
to
+348
| #[cfg(test)] | ||
| mod tests { | ||
| use super::{attr_item, directive_item}; | ||
| use tower_lsp::lsp_types::Documentation; | ||
|
|
||
| #[test] | ||
| fn directive_completion_docs_include_highlightable_example() { | ||
| let item = directive_item("v-if", "Conditional rendering", "v-if=\"$1\""); | ||
| let doc = markdown_doc(&item.documentation); | ||
|
|
||
| assert!(doc.contains("**Example**"), "got {doc:?}"); | ||
| assert!(doc.contains("```vue"), "got {doc:?}"); | ||
| assert!(doc.contains("Vue built-in directives"), "got {doc:?}"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn attribute_completion_docs_include_template_syntax_link() { | ||
| let item = attr_item("class", "CSS classes", "class=\"$1\""); | ||
| let doc = markdown_doc(&item.documentation); | ||
|
|
||
| assert!(doc.contains("Template attribute"), "got {doc:?}"); | ||
| assert!(doc.contains("```vue"), "got {doc:?}"); | ||
| assert!(doc.contains("Vue template syntax"), "got {doc:?}"); | ||
| } | ||
|
|
||
| fn markdown_doc(doc: &Option<Documentation>) -> &str { | ||
| match doc.as_ref().expect("completion should include docs") { | ||
| Documentation::MarkupContent(content) => &content.value, | ||
| Documentation::String(value) => value, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consider extending doc-content assertions to remaining item constructors.
Only directive_item and attr_item get dedicated Markdown-content tests; vize_directive_item, component_item, api_item, macro_item, and css_item were also migrated but aren't directly asserted here.
🤖 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 `@crates/vize_maestro/src/ide/completion/items.rs` around lines 317 - 348, The
Markdown documentation assertions only cover directive_item and attr_item, so
the other migrated completion constructors are not verified. Extend the tests in
tests to add coverage for vize_directive_item, component_item, api_item,
macro_item, and css_item, and assert their documentation content through
markdown_doc so each constructor’s docs are checked for the expected Markdown
formatting and links.
Comment on lines
+342
to
+347
| fn markdown_doc(doc: &Option<Documentation>) -> &str { | ||
| match doc.as_ref().expect("completion should include docs") { | ||
| Documentation::MarkupContent(content) => &content.value, | ||
| Documentation::String(value) => value, | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
markdown_doc test helper duplicated across multiple files.
The same helper appears verbatim in native_tests.rs, template_event_tests.rs, and (per the supplied context) component_props_tests.rs. Consider extracting to a shared test util module to avoid drift.
🤖 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 `@crates/vize_maestro/src/ide/completion/items.rs` around lines 342 - 347, The
markdown_doc test helper is duplicated across multiple test files, so extract
the shared logic into a common test utility module and update callers to use it.
Move the helper currently defined in markdown_doc so native_tests.rs,
template_event_tests.rs, and component_props_tests.rs all reference the same
function, keeping the Documentation matching behavior identical in one place to
prevent drift.
Comment on lines
+1
to
+2
| //! Markdown docs for component prop and slot completions. | ||
| #![allow(clippy::disallowed_macros)] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Module-wide clippy suppression is broader than the established pattern.
#![allow(clippy::disallowed_macros)] is applied at file scope here, whereas the referenced markup.rs module applies #[allow(clippy::disallowed_macros)] per-function. File-level suppression silences the lint for any future code added to this file, not just the format!-using helpers here.
♻️ Scope the allow to the functions that need it
-//! Markdown docs for component prop and slot completions.
-#![allow(clippy::disallowed_macros)]
+//! Markdown docs for component prop and slot completions.
use tower_lsp::lsp_types::Documentation;
use crate::ide::markup::Markdown;
+#[allow(clippy::disallowed_macros)]
pub(super) fn prop_documentation(🤖 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 `@crates/vize_maestro/src/ide/completion/template/component_docs.rs` around
lines 1 - 2, The clippy suppression in component_docs is too broad because it is
applied at module scope, which will hide future disallowed-macros uses in this
file. Remove the file-level `#![allow(clippy::disallowed_macros)]` and scope the
allowance only to the specific helper functions in `component_docs` that need
it, following the same per-function pattern used in `markup.rs`.
Comment on lines
+39
to
47
| /// Add a syntax-highlightable example block. | ||
| #[allow(clippy::disallowed_macros)] | ||
| pub fn example(mut self, language: &str, code: &str) -> Self { | ||
| self.sections.push(format!( | ||
| "**Example**\n\n{}", | ||
| markup::code_block(language, code) | ||
| )); | ||
| self | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Reuse self.section(...) instead of re-hardcoding the heading format.
example() and docs() manually rebuild the exact "**{heading}**\n\n{body}" pattern that section() (Line 51-54) already implements — and that markup::Markdown::example/docs also implement independently. This duplicates formatting logic in two places; if the heading format ever changes, one implementation can drift from the other. Delegating to self.section(...) also removes the need for the extra #[allow(clippy::disallowed_macros)] on these two methods.
♻️ Proposed refactor
/// Add a syntax-highlightable example block.
- #[allow(clippy::disallowed_macros)]
pub fn example(mut self, language: &str, code: &str) -> Self {
- self.sections.push(format!(
- "**Example**\n\n{}",
- markup::code_block(language, code)
- ));
- self
+ self.section("Example", &markup::code_block(language, code))
} /// Add a named documentation link section.
- #[allow(clippy::disallowed_macros)]
pub fn docs(mut self, text: &str, url: &str) -> Self {
- self.sections
- .push(format!("**Docs**\n\n{}", markup::link(text, url)));
- self
+ self.section("Docs", &markup::link(text, url))
}Also applies to: 85-91
🤖 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 `@crates/vize_maestro/src/ide/hover/builder.rs` around lines 39 - 47, The
example/docs formatting is duplicated by manually rebuilding the same
heading/body pattern instead of reusing the existing section helper. Update
HoverBuilder::example and the matching docs method to delegate through
self.section(...) (or a shared helper) so the "**{heading}**\n\n{body}" logic
lives in one place, and remove the now-unnecessary allow on those methods.
Comment on lines
+87
to
+98
| while cursor < snippet.len() { | ||
| let rest = &snippet[cursor..]; | ||
| if let Some(stripped) = rest.strip_prefix('$') | ||
| && let Some(ch) = stripped.chars().next() | ||
| { | ||
| if ch.is_ascii_digit() { | ||
| if ch != '0' { | ||
| output.push_str("..."); | ||
| } | ||
| cursor += 1 + ch.len_utf8(); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no current snippet definitions use bare multi-digit tabstops (e.g. $10+)
rg -nP '\$\d{2,}(?!\{)' crates/vize_maestro/src --type rust -C2Repository: ubugeeei-prod/vize
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== markup.rs around snippet_for_docs ==\n'
sed -n '1,220p' crates/vize_maestro/src/ide/markup.rs
printf '\n== Search for snippet syntax in crate ==\n'
rg -n '\$\{?[0-9]+' crates/vize_maestro/src crates/vize_maestro/tests -g '!target' || true
printf '\n== Search for snippet_for_docs callers ==\n'
rg -n 'snippet_for_docs|snippet\(' crates/vize_maestro/src -g '*.rs' || trueRepository: ubugeeei-prod/vize
Length of output: 20328
Consume the full bare tabstop index in snippet_for_docs.
$10 is currently rendered as ...0 because the bare $N branch only advances past one digit. Bare tabstops allow multi-digit indices, so this should consume the full digit run and collapse it to ... like the braced form.
🤖 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 `@crates/vize_maestro/src/ide/markup.rs` around lines 87 - 98, The bare tabstop
handling in snippet_for_docs only consumes a single digit after $, so
multi-digit placeholders like $10 are rendered incorrectly. Update the cursor
advance in the snippet parsing loop in markup.rs to consume the full consecutive
digit run for the bare $N case, matching the braced tabstop behavior, and keep
the existing collapse-to-... logic for any nonzero tabstop index.
Comment on lines
+93
to
+95
| assert.match(clickHoverText, /\*\*Example\*\*/); | ||
| assert.match(clickHoverText, /```vue/); | ||
| assert.match(clickHoverText, /Vue Event Handling/); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Assertions target markdown formatting, not observable behavior.
Checking for **Example** and ```vue tokens tests incidental formatting rather than the actual hover content (e.g., whether the expected event-handling snippet is present). This makes the test brittle to cosmetic heading/label changes and doesn't validate meaningful behavior. Consider asserting on the actual example content (e.g., the handler snippet text) instead of just the surrounding markdown markers.
As per path instructions, "Ensure workflow and release tests assert observable behavior rather than incidental formatting. Prefer focused assertions that keep PRs small and failures easy to diagnose."
🤖 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 `@tests/tooling/lsp-authoring-core.test.ts` around lines 93 - 95, The hover
test is asserting markdown wrappers instead of the real behavior, making it
brittle to cosmetic wording changes. Update the assertions around clickHoverText
in the lsp-authoring-core test to check for the actual example/handler snippet
content that should appear in the hover, rather than the **Example** label or
vue fence markers. Keep the checks focused on observable hover text from the Vue
event-handling example so failures reflect real regressions, not formatting
churn.
Source: Path instructions
Comment on lines
+165
to
+166
| assert.match(disabledText, /\*\*Example\*\*/); | ||
| assert.match(disabledText, /```vue/); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Same formatting-focused assertion concern as in lsp-authoring-core.test.ts.
These new checks assert on markdown markers (**Example**, ```vue) rather than the actual example content, which is incidental formatting per the path instructions for this directory.
As per path instructions, "Ensure workflow and release tests assert observable behavior rather than incidental formatting. Prefer focused assertions that keep PRs small and failures easy to diagnose."
Also applies to: 176-177
🤖 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 `@tests/tooling/lsp-editor-production-gates.test.ts` around lines 165 - 166,
The assertions in the gating tests are too formatting-specific; update the
checks in the affected test cases to verify the actual example content or
observable disabled/enabled behavior instead of markdown markers like bold
headings or fenced code blocks. Use the existing test block in
lsp-editor-production-gates.test.ts to locate the impacted assertions and
replace them with focused expectations that validate the semantic output from
the example content.
Source: Path instructions
Contributor
Tool BenchmarkMeasured: 2026-07-08T21:17:31.837Z
Fairness notes:
Commands: gh workflow run tool-benchmark.yml --ref <branch> -f file_count=3000 -f check_file_count=500 -f vite_file_count=1000 -f nuxt_file_count=250 -f large_blocks=300 -f runs=3 -f warmups=1 -f commit_results=true
node bench/generate.mjs 3000
node bench/compare-tools.mjs --input bench/__in__ --vize-bin target/release/vize --runs 3 --warmups 1 --check-file-count 500 --vite-file-count 1000 --nuxt-file-count 250 --large-blocks 300 --runner-label "blacksmith-32vcpu-ubuntu-2404" --out tool-benchmark-summary.md --json tool-benchmark-results.json --doc performance-blacksmith.mdVariant details and raw run timesSFC compile
Large SFC compile
Large SFC type check
Lint
Format
Type check
Vite build (end-to-end)
Nuxt SPA build (end-to-end)
|
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
Validation
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Bug Fixes