chore(docs): keep latest patch per minor in multi-version builds - #7096
chore(docs): keep latest patch per minor in multi-version builds#7096rh-hemartin wants to merge 1 commit into
Conversation
PR Summary by QodoKeep latest patch per minor in multi-version docs builds
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
|
🤖 Finished Review · ✅ Success · Started 9:56 AM UTC · Completed 10:11 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.57 |
Code Review by Qodo
1. Docs builds omit valid release minors
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Site previewPreview: https://84231f20-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Risk Assessment: moderate (2/5) DetailsRe-review confirms prior moderate score of 2: Tier 1 signals unchanged (6 files, 0 protected paths, 2 dependency files, non-bot non-first-time author), composite 2.1; Tier 2 moderate at 2.8 driven by high churn and author diversity on config.ts and package files offset by two brand-new files with no history; Tier 3 low at 1.7 reflecting tight issue-to-PR alignment and rollback safety; weighted composite (50%×2.1 + 30%×2.8 + 20%×1.7)=2.23 rounds to 2. Previous runRisk Assessment: moderate (2/5) DetailsRe-review confirms prior moderate score: Tier 1 signals unchanged (same file count, protected paths, dependency files, author type), composite of 2.10 rounds to 2. Well-scoped docs-infrastructure change with new utility and tests aligned to issue #7071; moderate risk driven by two dependency files touched and high author diversity/churn on config.ts and package files, offset by no security, CI, or protected-path exposure and strong issue alignment. Previous run (2)Risk Assessment: moderate (2/5) DetailsSmall, well-scoped docs-infrastructure change with test coverage and clear issue alignment; risk is moderate primarily due to two dependency files being touched and high author diversity in the docs area, but no security, CI, or protected-path exposure. |
|
Looks good to me Previous runReviewFindingsMedium
Low
Next steps:
Previous run (2)ReviewFindingsLow
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.
18eddce to
1fa3a4d
Compare
|
🤖 Finished Review · ✅ Success · Started 12:14 PM UTC · Completed 12:29 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.26 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
mvb tests each git tag with semver.satisfies independently, so a plain range includes every patch. Build a caret range-set from the latest qualifying patch of each minor instead. Signed-off-by: Hector Martinez <hemartin@redhat.com>
1fa3a4d to
e53f0f6
Compare
|
🤖 Finished Review · ✅ Success · Started 8:35 AM UTC · Completed 8:52 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.90 |
Superseded by updated review
fullsend-ai-review
Bot
added
the
ready-for-merge
| const MVB_TAG_MATCH = "v[0-9].*"; | ||
|
|
||
| /** Git tags mvb will later re-test with `semver.satisfies` (one version at a time). */ | ||
| function gitVersionTags(match: string): string[] { |
There was a problem hiding this comment.
[MEDIUM] Tag enumeration runs before mvb fetches tags — a stale local clone now ships the OLD patch and drops the newest
Verified against @lando/vitepress-theme-default-plus@1.2.0 (the pinned version). bin/mvb.js resolves the site config FIRST (line 48: const siteConfig = await resolveConfig(osource, 'build', 'production'), reading multiVersionBuild.satisfies at line 60) and only AFTER that copies the repo into tmpDir and fetches tags (line 123: ['fetch', 'origin'|'--all', '--tags', '--no-filter', '--force'], plus --unshallow at line 125). mvb's own tag enumeration (getTags(options.tmpDir, options), line 148) therefore sees the post-fetch tag set, while this PR's satisfies value is computed pre-fetch from whatever git tag --list returns in the local clone.
Consequence: if the local clone has v0.42.0 but not yet v0.42.1, getLatestPatchMatching pins the range-set to ... || 0.42.0 || ...; mvb then fetches v0.42.1 into tmpDir and get-tags.js rejects it (semver.satisfies('0.42.1', '…|| 0.42.0 ||…') is false). The build ships the OLDER patch, omits the newest, and stable (get-tags.js line 51, first non-prerelease of the rsorted list) regresses to it. Before this PR the same stale clone built both, because >=0.37.0 was evaluated against the post-fetch list.
CI is safe: .github/workflows/site-build.yml checks out with fetch-depth: 0 and fetch-tags: true. The exposure is npm run docs:build, which is git submodule update --init && mvb docs — no tag fetch. The latest.length > 0 ? … : range fallback in mvb-satisfies.ts:26 only covers an empty list, never a partial one, so this degrades silently.
Distinct from the already-posted thread on gitVersionTags error handling (discussion_r3958967731), which is about git failing, not about the list being stale-but-valid.
Suggested fix: Fetch tags ahead of config evaluation in the build entrypoint rather than inside config.ts (which is also loaded by vitepress dev, where a fetch would be unwanted): change the docs:build script to git submodule update --init && git fetch --tags --force && mvb docs, and note the requirement in docs/doc-site.md next to the new paragraph.
| multiVersionBuild: { | ||
| satisfies: ">=0.37.0", | ||
| match: MVB_TAG_MATCH, | ||
| satisfies: getLatestPatchMatching(gitVersionTags(MVB_TAG_MATCH), ">=0.37.0"), |
There was a problem hiding this comment.
[MEDIUM] Superseded patch dirs stop being built, but per-build /v/ listings are computed from each ref's own config and will link to them
The new range-set only decides WHICH versions mvb builds; it does not drive the version list rendered inside each build. Verified chain: docs/v/index.md calls useTags() -> the theme's client/tags.data.js, which reads globalThis.VITEPRESS_CONFIG.userConfig.themeConfig.multiVersionBuild and calls get-tags.js with it. I confirmed mvb does NOT propagate the parent's satisfies to sub-builds: bin/mvb.js sets only VPL_MVB_BASE / VPL_MVB_BUILD / VPL_MVB_DEV_VERSION / VPL_MVB_BRANCH / VPL_MVB_SOURCE (lines 234-238), and utils/normalize-mvb.js reads no env for satisfies or match. So every build's /v/ page is generated from the multiVersionBuild.satisfies of the ref that build checked out (bin/mvb.js line 212, git checkout ref).
Two consequences:
(a) The root build is the checkout of the stable tag (build: "stable"; extended.unshift of the stable alias, mvb.js line 158). Until a tag containing this PR is cut, stable is v0.43.0, whose tree still has satisfies: ">=0.37.0" — so the live /docs/v/ page will list every patch of a minor while the parent build only emits one dir per minor.
(b) Worse and durable: site-build.yml caches each versioned build under docs/.vitepress/cache/@lando/mvb, and mvb restores from cachePath and never rebuilds a cached tag (mvb.js lines 197-201). The cache key is base/versionBase/version/base hashed (line 174) — it contains NO hash of the config or of the version set, so a cached per-version /v/ page is frozen at first-build time forever. In the backport scenario the linked issue itself uses as its example (v0.42.1 tagged after v0.43.0 is stable), every already-cached build whose /v/ page listed v0.42.0 keeps linking to /docs/v/v0.42.0/, which the new build set no longer produces (each build's outDir is removed and re-copied, mvb.js line 199 / 250).
Those links then fall through to the Cloudflare 404.html; I grepped cloudflare_site/worker/ and found no redirect handling for /docs/v/ paths. Today's tag set (v0.37.0, v0.38.0, v0.39.0, v0.40.0, v0.41.0, v0.42.0, v0.43.0) has no duplicate patches, so nothing breaks yet — this is latent, triggered by the first patch release, which is exactly the case the PR exists to handle. Neither the PR body nor the one-line docs/doc-site.md change mentions the trade-off.
Suggested fix: At minimum, document the consequence in the new docs/doc-site.md paragraph: older cached builds — and the root build until the next tag is cut — still list superseded patch dirs that are no longer emitted. If dangling links are unacceptable, either add a worker-side redirect from a superseded /docs/v/vX.Y.Z/* to the built patch of that minor (the built set is derivable from the same range-set), or keep superseded patch dirs in dist and drop them only from the listing. Busting the mvb cache key when the computed range-set changes would also force stale per-version listings to regenerate.
2 participants
Summary
Keep only the latest patch of each minor in the docs multi-version build.
mvbcurrently ships every tag that satisfies>=0.37.0, so bothv0.42.0andv0.42.1appear once both exist.Related Issue
Fixes #7071
Changes
getLatestPatchMatchingto turn the existing floor into a caret range-set (^0.37.0 || ^0.38.1 || …) sosemver.satisfiesper tag rejects older patches of the same minorMVB_TAG_MATCH(v[0-9].*) betweengit tag --listandmultiVersionBuild.matchsemveras a direct dependency and unit tests for the range builderdocs/doc-site.mdThe
>=0.37.0floor is unchanged.Testing
make lintpasses (stage changes first, then run)Checklist
!for breaking changes)