Repository navigation
ci: publish a Python documentation preview for each pull request - #370
Merged
Merged
Conversation
Contributor
|
Preview root: https://posit-dev.github.io/commons/pr-370/ Python site preview: https://posit-dev.github.io/commons/pr-370/py/ Built from the latest commit on this branch. The R links in it point at the published R site, which no pull request rebuilds. |
jat255
added this pull request to stack #371
September 13, 2026 22:47
This was referenced Sep 13, 2026
jat255
added
documentation
jat255
force-pushed
the
jat255/qp8j-docs-previews
branch
from
September 13, 2026 23:07
b58767a to
c92bf12
Compare
Contributor
|
Preview deployed to Connect ( Deployed from commit 9134b0e. |
Contributor
|
Preview deployed to Connect ( Deployed from commit 9134b0e. |
Both site workflows already built on pull_request and skipped only the deploy step, so a preview is that same deploy with target-folder pr-<number> and the condition inverted. Each posts a sticky comment with the link. Fork pull requests are excluded: their token is read-only, and the alternative means serving unreviewed fork HTML from posit-dev.github.io. Preview folders are collected on pull request close and, more importantly, by a weekly job that lists pr-* on gh-pages and keeps only the folders whose pull request is still open. Event-driven cleanup is best-effort, so the schedule is what makes the branch converge from any state. It asks for the open pull requests once rather than per folder: a per-folder query has to decide what a failed query means, and treating failure as "not open" would delete every live preview the first time the API rate-limited us. The deploy concurrency group is now shared by every run that pushes to gh-pages, including previews, which previously could not collide because pull requests did not deploy. robots.txt keeps previews out of search results, since the repository is public.
The site workflows skip the preview deploy for forks, and a fork's token is read-only, so they cannot explain the skip themselves. pull_request_target runs with the base repository's token and can. The job checks out nothing and holds only pull-requests: write, so it neither reads nor runs fork code.
Review caught two defects in the preview workflows. Serializing every gh-pages push through one concurrency group was wrong. Actions keeps only one pending run per group, so a third arrival cancels the queued one outright: a pull request touching both packages, landing alongside a push to main, would lose one site's preview with no failure to show for it. Groups are per workflow and per pull request again, and the races they were meant to prevent are handled where they happen, by the deploy action's rebase. That needs force: false, since the default force-push would drop whichever deploy landed first. The cleanup job rebases and retries its own push for the same reason. robots.txt did nothing. This is a project site under posit-dev.github.io, and crawlers read robots.txt only at the origin root, which this repository does not own. Preview pages now carry a noindex meta tag instead, added by a script both workflows run before the preview deploy. Shipping a robots.txt that looks like protection and is never read is worse than shipping nothing, so it is gone.
The head start tag is optional in HTML, so reporting a page that lacks one and moving on left it deployed and indexable. The script synthesizes a head now, after the html tag where there is one and at the top of the file otherwise. Failing the run instead, as the review suggested, would let a single vendored fragment block every preview, and a fragment is usually not a page anyone would index. Neither generator omits the tag today: all 70 pages across the two built sites carry one.
A document may omit both <html> and <head> but still open with a doctype, and a doctype only counts if nothing precedes it. Prepending the synthesized head demoted it and put the browser in quirks mode, so the preview would render unlike the page it previews. The head now goes after the doctype when there is no <html> tag to follow, and only a file with neither gets it prepended.
Three review rounds found three defects in this script, all in the branch that handles a page with no head, and none of them reachable from either generator: all 70 pages across the two built sites carry a head. The branch was untested because a .github/scripts helper has no package to be tested by. Removed the fallback that prepended a head to anything at all. A file with no head, no html tag, and no doctype is a fragment, not a page, so nothing indexes it alone and giving it a head only corrupts whatever embeds it. Skipping it removes the ordering question the last two defects were about. The doctype pattern now also allows a leading byte order mark, which is the remaining way a doctype could have been demoted. The transformation is a pure function now, with tests covering an existing head, an uppercase head with attributes, a page that already declares robots, head synthesis after an html tag, doctype-first in the upper, lower, and byte order mark forms, fragments, and idempotency. A small workflow runs them, since the package suites are scoped to their own directories.
The doctype alternative was not anchored, so any file quoting a doctype anywhere passed as a document and was rewritten. It is anchored now, and the byte order mark is written as an escape rather than as the literal character, which was invisible in the source. Review did not mention the related defect, which the same regex caused: with both alternatives in one pattern, the leftmost match won, so a page with a doctype and an html tag but no head had its head inserted between the two, outside html. The html tag is now tried first, and the doctype only when there is no html tag to follow. Both cases have tests. __pycache__/ was ignored only under pkg-py, so running the new test left a bytecode cache that a directory-wide git add would pick up. The root file's own convention is that a pattern with no separator belongs at the root and applies at any depth.
The sites are published as one tree, and their links to each other are relative, so opening a built file directly resolves none of them. scripts/preview-docs.sh assembles the same tree a deploy publishes and serves it, so a local look and a pull request preview show the same thing. It runs each package's real build commands rather than its own, so the output matches what CI produces. A missing toolchain skips that site with a line naming what to install, and the script says which site is absent, because the alternative is clicking a cross-language link and wondering whether the build broke. Requiring both toolchains would make it useless to anyone set up for one language. The R site needs commons installed, as the workflow does through setup-r-dependencies, so the script checks for it up front. Without the check a missing install surfaces as an R stack trace several minutes into a build.
Naming the missing package left the reader to work out where to type the fix, and the answer differs between the shell and the R console. Each skip now prints a reason and a command to paste. The commons one installs pak if it has to and runs from the repository root, so it works from whatever directory the reader happened to be in. The R site's real blocker on a fresh machine is magick, an Imports with a macOS binary on CRAN, so nothing outside R needs installing. Letting local_install_deps read DESCRIPTION covers it without naming it here, where it would go stale.
One long wrapped line between two other messages is not something anyone copies. Each skip now prints its reason, then its commands one per line, indented with nothing in front of them so the block can be selected and pasted whole. No box drawing: the border characters come along with the copy. The commands take an absolute path rather than opening with cd, so pasting them does not move the reader's shell, and the pak line appears only when pak is actually missing instead of as a conditional to evaluate by eye.
pkgdown's automatic development mode decides the destination from the version in DESCRIPTION: a released version lands at the root of destination, a development version in dev/ below it. DESCRIPTION is at 0.1.0.9000, so a build from this branch produces only docs/r/dev/, and both the preview comment and the local script pointed at docs/r/, which serves a directory listing. The deployed site hides this, because v0.1.0 was released and left an index there. Both now resolve the entry point from what the build produced. The local script prints the address of each site after building rather than leaving the reader to infer it from the landing page, whose r/ link is correct for the deployed site and wrong for a development build.
A build from a branch is always a development version, so pkgdown writes it to r/dev/ and leaves r/ empty, and the landing page's link to r/ lands on nothing. Moving dev/ up would fix the link and break the site: the build writes its own absolute address into search.json, sitemap.xml, and 15 pages, so a moved tree keeps directing the browser to r/dev/, and site search stops resolving. A redirect at r/index.html leaves the build untouched and costs one file. It is written for local previews and pull request previews only. The published site keeps the released documentation at r/, and replacing that index with a redirect to the development docs would be a regression for every reader. The production deploy step and the preview steps are mutually exclusive on the event, so the two cannot cross.
pkgdown passes --mathml once per topic and rmarkdown passes --mathjax once per vignette; pandoc 3.11 deprecated both spellings, and neither flag is reachable from here. Filter those lines out of the R build's stderr, keeping stdout and the exit status untouched.
The icon set pkgdown generated was never committed, so only a local R build had a favicon. Move it to favicon/ at the repository root, sync it into the pkgdown directory, the Quarto site, and the landing page, and give the Python site and the landing page the same five link tags pkgdown writes. The manifest named its icons with absolute paths, which resolve above a site published under a prefix, and carried no name. Destinations marked :bare skip the sync script's generated README, since their contents are copied onto a website as they stand.
jat255
force-pushed
the
jat255/qp8j-docs-previews
branch
from
September 13, 2026 23:24
c92bf12 to
2e48142
Compare
simonpcouch
reviewed
Sep 14, 2026
| github.event_name == 'pull_request' && | ||
| github.event.pull_request.head.repo.fork != true | ||
| working-directory: . | ||
| run: .github/scripts/link-r-dev.sh docs |
Collaborator
There was a problem hiding this comment.
re:
One little wrinkle/oddity is the interaction with pkgdown's automatic development mode. Because the deployed site expects the R site at /r, but pkgdown writes into /r/dev, the link-r-dev.sh script was necessary to provide a shim that writes a redirect into r/index.html only when that file does not exist (this is true for a development version and false for a released one). If the R package ever builds a preview from a released version, the redirect correctly does not appear. I'm not overly familiar with the benefits of pkgdown's dev mode, but I'm wondering if it's causing more trouble than value here.
It does provide a good bit of value here, and I'd prefer we keep it as-is. It prevents accidentally changing the production version of the website unless you've actually sent a new version to CRAN—we assume that most people use the CRAN version, so we keep the site pinned at that version. I can make the update-released-site-with-a-dev-version change happen after merge of the other PR, but I'd prefer we keep this as-is. Otherwise, we'll get a /dev version of the site when we merge to main, but it's personally not important to me to have deployed documentation previews per-PR.
The R site now follows the usual R package arrangement: pkgdown's automatic development mode publishes the development version to r/dev on each push to main, and r/ holds the released documentation, redeployed by hand at release. No pull request builds a preview of it. The pkgdown workflow still builds the site on a pull request, as a check that it builds. The Python site keeps its preview, so the shared pieces stay: noindex-preview, the preview cleanup, and the fork notice. With no R site in a preview, the links into it are made absolute, matching what the R site's Python link already does. link-r-dev.sh existed only to make the landing page's relative r/ link resolve in a preview, and goes with it; the local preview script now points at r/dev where pkgdown actually writes it.
jat255
changed the title
The per-pull-request group was for the preview deploys, which are gone. It was also worse than what it replaced: a push to main and a release tag get different refs, so the two deploys no longer serialized against each other. The usethis form groups every non-PR run together and leaves PR builds ungrouped, which is what this workflow now wants.
Contributor
|
Cleaned up 4 preview bundle(s) on https://dogfood.team.pct.posit.it: 370345, 370348, 370610, 370612 |
Contributor
|
Cleaned up 4 preview bundle(s) on https://connect.staging.pct.posit.it: 2903, 2906, 2911, 2912 |
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.
This PR adds a documentation preview for each pull request that rebuilds the Python site. It publishes the landing page and the Python site as one tree under
pr-<number>/on thegh-pagesbranch, and then comments a link to it. A cleanup action removes the tree when the pull request closes.Per @simonpcouch's request, the R site does not get a preview build, and keeps the usual R package arrangement:
r/dev/holds the development version (i.e.main),r/holds the released documentation, andr/is redeployed by hand at release.The PR also adds a helper script,
scripts/preview-docs.shto build and serve both sites locally, and gives all three sites one shared favicon.Stacked on #366. This replaces #368, which was accidentally merged and then unwound.
Implementation notes (agent-written)
The rationale for each piece is in a comment at the top of the file, so this only says where to look.
Publishing.
quartodoc.yamlgains a preview deploy alongside the existing one.noindex-preview.pyadds arobotsmeta tag to every page, because arobots.txtunderposit-dev.github.iois never read.pkgdown.yamlstill builds the R site on a pull request, as a check that it builds, but deploys nothing there.Cross-site links. A preview contains the landing page but no R site, so the links into the R site are absolute in
docs/index.htmland_quarto.yml. They point at the published site from everywhere. This matches what_pkgdown.ymlalready does for its Python link, which has to survive being served from bothr/andr/dev/.Cleanup.
preview-cleanup.ymlremoves a preview when its pull request closes, and a weekly schedule collects whatever the close event missed. The schedule cannot run until this merges, so the manual run defaults to a dry run.Forks. A fork gets no preview, because building one would serve unreviewed HTML from
posit-dev.github.io.preview-fork-notice.ymlsays so in a comment. It usespull_request_targetbecause a fork's token cannot write one; it checks out no code and runs none.Favicon.
favicon/at the repository root is the source, andsync-shared.shcopies it into the two packages and the landing page. Destinations marked:bareskip the generated README, because their contents go onto a website as they stand.Tests.
.github/scripts/belongs to no package, soscripts-check.yamlrunstest_noindex_preview.pyon it.Verification.
scripts/preview-docs.shbuilt both sites and served them. The landing page,r/dev/, andpy/all resolved, and their links to each other worked.scripts/sync-shared.shregenerates every destination with no drift.R changes
Two things change on the R side:
Favicon assets are now copied by
scripts/sync-shared.shintopkg-r/pkgdown/favicon/, which pkgdown picks up when it builds the site. The only visible difference is that the R site serves the same icons as the other two.pkgdown.yamlhas a few changes to manage concurrency between itself andquartodoc.yaml, so race conditions between different builds don't clobber the status of thegh-pagesbranch.