refactor(docs): switch to VitePress Plus theme with multi-version builds - #6010
refactor(docs): switch to VitePress Plus theme with multi-version builds#6010rh-hemartin wants to merge 1 commit into
Conversation
|
🤖 Finished Review · ✅ Success · Started 1:59 PM UTC · Completed 2:19 PM UTC Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ReviewFindingsMedium
Low
Next steps:
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsMedium
Low
Next steps:
Previous run (3)ReviewFindingsMedium
Low
Next steps:
Previous run (4)ReviewFindingsMedium
Low
Next steps:
Previous run (5)ReviewFindingsMedium
Low
Next steps:
Previous run (6)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (7)ReviewFindingsMedium
Low
Previous run (8)ReviewFindingsMedium
Low
Labels: PR modifies .github/workflows/site-build.yml (CI pipeline for doc builds) Previous run (9)ReviewFindingsMedium
Low
Previous run (10)ReviewFindingsMedium
Low
Previous run (11)ReviewFindingsMedium
Low
Next steps:
Previous run (12)ReviewFindingsMedium
Low
Previous run (13)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (14)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (15)ReviewFindingsHigh
Medium
Low
Labels: PR modifies VitePress documentation infrastructure and adds multi-version doc support. Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
399214e to
30dffbb
Compare
|
🤖 Review · Commit: |
Not true, it resolves correctly as far as I can see ( |
30dffbb to
0b09c0f
Compare
Done |
|
🤖 Review · Commit: |
Done |
0b09c0f to
871c3a6
Compare
|
🤖 Review · Commit: |
871c3a6 to
2bbad0d
Compare
Everything working well, so no problem here. |
It does handle escaping, but not everything weird we may throw at it. We need to scape those strings and there are not even that many. Rejected. |
|
🤖 Review · Commit: |
2bbad0d to
a6f53d7
Compare
|
🤖 Review · ❌ Terminated · Started 7:59 AM UTC · Ended 8:19 AM UTC Commit: |
Site previewPreview: https://cef2cd01-site.fullsend-ai.workers.dev Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 7:59 AM UTC · Completed 8:19 AM UTC Commit: |
|
🤖 Review · ❌ Terminated · Started 8:46 AM UTC · Ended 9:03 AM UTC Commit: |
PR Summary by QodoDocs: adopt VitePress Theme+ with multi-version builds (mvb)
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
🤖 Finished Review · ✅ Success · Started 2:52 PM UTC · Completed 3:13 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.67 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Local npx mvb docs at bc63d66 succeeded (2 builds: primary dev at /docs/ plus /docs/v/dev/). CI Build Site is green.
Remaining review-bot items are not blocking:
- env-var:
mvbsetsVPL_MVB_BRANCHper sub-build (see themebin/mvb.js);config.tsreading that is correct.docs/doc-site.mdis the stale side. satisfies: ">=0.38.0"intentionally skips every current tag (newest isv0.37.0), so onlydevis built until 0.38 ships. Matches the earlier "don't rebuild historical tags under this theme" fix./docs/→/docs/latest/is not in this tree; theme does not emit that redirect here. Fine until a stable alias exists.
Assisted-by: Claude
|
Follow-up on my approval: I verified the built site with playwright-cli against the preview at The sidebar version ender shows the raw 40-char head SHA instead of It is also clipped: the label measures 349px inside a 340px sidebar, so it cuts off mid-SHA. Same on the versioned sub-build This is not preview-only. env:
VPL_MVB_BRANCH: ${{ github.event.pull_request.head.sha || github.sha }}On push to Chain: workflow env → Suggested fixes:
Secondary, non-blocking: mvb rewrites the
Everything else on the built site checks out: Assisted-by: Claude |
Rescinding: verified the built preview with playwright-cli and the sidebar version label renders the raw 40-char head SHA instead of a version, and it is not preview-only — it will follow to production after merge. Details: #6010 (comment)
bc63d66 to
512d332
Compare
|
@waynesun09 inspecting the code looks like the theme rewrites package.json on the temporal folder with a correct version, so the right move is to retrieve the version from package.json, even if we don't set it. I also added cache stuff. |
|
🤖 Review · Commit: |
512d332 to
98ad03d
Compare
|
🤖 Review · Commit: |
98ad03d to
809fdb5
Compare
|
🤖 Finished Review · ✅ Success · Started 7:33 AM UTC · Completed 7:51 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.36 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 2:44 PM UTC · Completed 3:03 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.02 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 7:13 AM UTC · Completed 7:54 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.97 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Replace the default VitePress theme with @lando/vitepress-theme-default-plus and enable multi-version documentation builds via mvb. Adopt the Lando defineConfig wrapper, add multiVersionBuild/sidebarEnder configuration, derive the sidebar version label from VPL_MVB_VERSION, simplify Vite resolve aliases with import.meta.resolve, and update CI to fetch tags for version discovery. Rework custom CSS for the new theme and preserve -webkit- vendor prefixes for Safari compatibility. Signed-off-by: Hector Martinez <hemartin@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Hector Martinez <hemartin@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 8:31 AM UTC · Completed 9:11 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.29 |
There was a problem hiding this comment.
Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:
.github/workflows/site-build.yml(file-level): Line 1 · [medium] scope-vs-authorization
The workflow uses path filters (on.push.paths and on.pull_request.paths) but does not include merge_group: as a trigger. Per docs/contributing/ci-workflows.md, path-filtered workflows must include merge_group: with a path-relevance guard step. This is a pre-existing gap not introduced by this PR, but surfaced because the PR modifies the workflow.
Suggested fix: Add merge_group: to the on: triggers and add a path-relevance guard step.
docs/.vitepress/theme/components/Mermaid.vue(file-level): Line 58 · [low] incomplete-formatting-within-file
Most of Mermaid.vue reformatted to double-quote + semicolon style, but the openLightbox and onFigureClick functions still use single quotes and omit semicolons.
Suggested fix: Run prettier on the whole file.
| @@ -15,6 +15,10 @@ import { | |||
| const __dirname = path.dirname(fileURLToPath(import.meta.url)); | |||
| const docsDir = path.resolve(__dirname, ".."); | |||
|
|
|||
There was a problem hiding this comment.
[low] runtime-mechanism
The version variable reads .version from package.json, which has no version field in the repo. The ?? "dev" fallback always triggers for local and PR CI builds. The mechanism is intentional: mvb injects the version into package.json during tagged-release sub-builds in a temp checkout, and doc-site.md documents this behavior.
| "docs:dev": "git submodule update --init && vitepress dev docs", | ||
| "docs:build": "git submodule update --init && vitepress build docs", | ||
| "docs:build": "git submodule update --init && mvb docs", | ||
| "docs:preview": "vitepress preview docs" |
There was a problem hiding this comment.
[low] edge-case
The docs:preview script (vitepress preview docs) serves a single VitePress build. After an mvb multi-version build, the output structure includes versioned subdirectories that vitepress preview may not correctly route, causing 404s for versioned paths.
| import type { UserConfig } from "vitepress"; | ||
|
|
||
| interface VPLThemeConfig { | ||
| sidebarEnder?: unknown; |
There was a problem hiding this comment.
[low] weak-type-declarations
sidebarEnder and multiVersionBuild are typed as unknown in the declaration file, plus a catch-all [key: string]: unknown index signature. This provides minimal type safety for the VPL theme config extensions.
Suggested fix: Define the shapes of sidebarEnder and multiVersionBuild to match their usage in config.ts.
2 participants
Summary
Closes #5717
@lando/vitepress-theme-default-plus, which provides built-in multi-version doc support via git-tag discovery and themvbCLIescapeVueSyntaxmarkdown preprocessor for most pages; skip it only forv/index.mdso Theme+ Vue components on that page can compiledocs/v/index.mdand a sidebar version switcherimport.meta.resolveinstead of manualpath.resolvechainsTest plan
npm run docs:devstarts without errors and renders the site with green brandingnpm run docs:buildcompletes (mvb discovers tags and builds versioned output){{ }}and non-HTML<tags>in markdown render correctly withescapeVueSyntaxstill applied (exceptv/index.md)/v/versions page lists discovered tags🤖 Generated with Claude Code