builds: add files changed overview to the build detail page#13060
Draft
ericholscher wants to merge 4 commits into
Draft
builds: add files changed overview to the build detail page#13060ericholscher wants to merge 4 commits into
ericholscher wants to merge 4 commits into
Conversation
Surface the file tree diff on the build detail page so the "Files changed" tab works for both pull request builds and normal version builds. Pull request builds keep comparing against the project's base version. Normal version builds now compare against the version's own previous build, using a manifest snapshot taken before the latest build overwrites the manifest. https://claude.ai/code/session_01TBAfbmqA3Gy3ejeYkyHV7q
get_diff already handles missing manifests; match the addons API (hosting.py), which calls the diff helpers directly. https://claude.ai/code/session_01TBAfbmqA3Gy3ejeYkyHV7q
2 tasks
Move the successful/finished check into get_diff_for_build so the view just calls it directly, instead of wrapping it in a helper. https://claude.ai/code/session_01TBAfbmqA3Gy3ejeYkyHV7q
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a “Files changed” diff to the build detail page by extending the file tree diff system to support comparing normal version builds against the previous build’s manifest (via a new manifest snapshot), while keeping existing behavior for PR builds (compare against a pinned base manifest snapshot).
Changes:
- Snapshot and persist a “previous manifest” for normal versions before overwriting the current manifest.
- Add
get_diff_for_build(build)helper to choose the correct diff baseline (PR base version vs. previous build for normal versions). - Pass
files_changed_diffinto the build detail view context and add tests for the new behaviors.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| readthedocs/projects/tasks/search.py | Snapshots the previous manifest before writing a new manifest for non-external versions. |
| readthedocs/filetreediff/init.py | Adds previous-manifest snapshot support and introduces get_diff_for_build. |
| readthedocs/builds/views.py | Exposes files_changed_diff to the build detail template context. |
| readthedocs/filetreediff/tests/test_filetreediff.py | Adds unit tests for previous-manifest snapshots and get_diff_for_build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+153
to
+157
| # For normal versions, snapshot the existing manifest before it gets | ||
| # overwritten, so the file tree diff can compare this build against the | ||
| # version's previous build. | ||
| if not self.version.is_external: | ||
| snapshot_previous_manifest(self.version) |
| version = build.version | ||
| if not version: | ||
| return None | ||
|
|
| if not base_version: | ||
| return None | ||
|
|
||
| return get_diff(current_version=version, base_version=base_version) |
- Gate get_diff_for_build to the version's latest successful build; per-version manifests can only represent the latest build, so older build detail pages no longer surface a misleading diff for whatever the latest build did. - Drop the get_diff(v, v) overload that conflated "compare version to itself" with "compare latest to previous"; the previous-build path now lives in an explicit helper invoked from get_diff_for_build. - Tolerate pruned Build rows by replacing Build.objects.get with .filter().first() in the manifest-build resolution; a stale manifest no longer crashes the page. - Skip the previous-manifest snapshot when the existing manifest already belongs to the current build (reindex_version, build reruns); refreshing it would clobber the real previous baseline and yield an empty diff. - Pin the snapshot/write ordering invariant with an inline comment and a test that asserts collect() calls snapshot_previous_manifest before write_manifest. - Deduplicate the base-version lookup into get_diff_base_version(). - Rename the misleading build_id log field on snapshot_previous_manifest to previous_build_id. Generated by an AI agent (Claude Code).
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
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
Exposes a file tree diff on the build detail page so it can be shown in a new "Files changed" tab. It works for both pull request builds and normal version builds, with the correct base to compare against:
BuildDetailpasses afiles_changed_diff(FileTreeDiff) to the template for successful, finished builds.The frontend that renders this tab lives in the companion PR: readthedocs/ext-theme#744.
Test plan
tox -e py312 -- -k "filetreediff or PreviousManifestSnapshot or files_changed or post_build_overview"— new and existing tests passThis pull request was generated by an AI agent (Claude Code).