Add visual diff base HTML snapshot for stable PR comparisons#12928
Draft
ericholscher wants to merge 2 commits intomainfrom
Draft
Add visual diff base HTML snapshot for stable PR comparisons#12928ericholscher wants to merge 2 commits intomainfrom
ericholscher wants to merge 2 commits intomainfrom
Conversation
When a PR is first built, copy the base version's *.html files into the
PR's diff storage (external/diff/{project}/{pr_slug}/base_html/). This
frozen snapshot ensures visual diff compares against the base state at
PR creation time, not the current tip of the base branch.
Key changes:
- Add rclone copy_remote() for S3-to-S3 server-side copies
- Add snapshot_base_html() to copy base HTML during PR post-build
- Add proxito route /_/diff/{version}/base/{path} to serve snapshots
- Update doc_diff.base_url in addons API to use snapshot when available
- Cleanup is automatic via existing PR version deletion flow
https://claude.ai/code/session_01MczHXuiDZUFE8TLFhV1xXs
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| doc_diff = r.json()["addons"]["doc_diff"] | ||
| # Should fall back to the live latest version URL. | ||
| assert "/en/latest/index.html" in doc_diff["base_url"] | ||
| assert "project.dev.readthedocs.io" in doc_diff["base_url"] |
- Fix import ordering in serve.py: move filetreediff import to alphabetical position among readthedocs.* imports - Fix test_snapshot_base_html: mock get_base_snapshot and write_base_snapshot directly instead of blanket storage mock that broke the write path https://claude.ai/code/session_01MczHXuiDZUFE8TLFhV1xXs
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
3 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.
Summary
doc_diff) remains stable when the base branch advances*.htmlfiles from the base version into the PR'sdiff/storage (external/diff/{project}/{pr_slug}/base_html/)/_/diff/{version}/base/{path}) to serve these frozen snapshotsdoc_diff.base_urlin the addons API to point to the snapshot URL for external versions, falling back to the live base URL when no snapshot existsDesign decisions
*.htmlfiles are copied (via rclone--include=*.html), skipping static assets — visual diff only needs the rendered HTML to extract<main>contentdiff/storage tree, so cleanup is automatic when the PR version is deleted (no reference counting needed)rclone copy_remote()method avoids downloading data to the builder; uses S3 CopyObject APIDepends on
Test plan
doc_diff.base_urlpoints to snapshot path for PR versions with a base snapshotrclone copy_remotewith--include=*.htmlonly copies HTML fileshttps://claude.ai/code/session_01MczHXuiDZUFE8TLFhV1xXs