Use exists() instead of parsing JSON for snapshot write-once check

The write-once guard in snapshot_base_manifest() was opening and JSON-parsing
the snapshot file just to check if it exists. Use build_media_storage.exists()
(S3 HEAD request) instead — cheaper and clearer intent.

https://claude.ai/code/session_01FgTyr1d9mpwms8emHGNAQf

Author: claude

readthedocs/filetreediff/__init__.py

@@ -177,17 +177,17 @@ def snapshot_base_manifest(external_version: Version, base_version: Version):
     manifests. Same approach could extend to all build artifacts if we move
     toward content-addressed storage.
     """
-    if _get_base_manifest_snapshot(external_version) is not None:
+    snapshot_path = external_version.get_storage_path(
+        media_type=MEDIA_TYPE_DIFF,
+        filename=BASE_MANIFEST_SNAPSHOT_FILE_NAME,
+    )
+    if build_media_storage.exists(snapshot_path):
         return
 
     base_manifest = get_manifest(base_version)
     if not base_manifest:
         return
 
-    snapshot_path = external_version.get_storage_path(
-        media_type=MEDIA_TYPE_DIFF,
-        filename=BASE_MANIFEST_SNAPSHOT_FILE_NAME,
-    )
     with build_media_storage.open(snapshot_path, "w") as f:
         json.dump(base_manifest.as_dict(), f)
 

readthedocs/filetreediff/tests/test_filetreediff.py

@@ -234,23 +234,9 @@ def test_snapshot_prevents_false_positives_from_stale_base(self, storage_open):
         assert diff.added == []
         assert diff.deleted == []
 
+    @mock.patch.object(BuildMediaFileSystemStorageTest, "exists", return_value=True)
     @mock.patch.object(BuildMediaFileSystemStorageTest, "open")
-    def test_snapshot_is_write_once(self, storage_open):
+    def test_snapshot_is_write_once(self, storage_open, storage_exists):
         """snapshot_base_manifest is a no-op if a snapshot already exists."""
-        existing = json.dumps(
-            {"build": {"id": 1}, "files": {"index.html": {"main_content_hash": "old"}}}
-        )
-
-        @contextmanager
-        def mock_existing(*a, **kw):
-            m = mock.MagicMock()
-            m.read.return_value = existing
-            yield m
-
-        storage_open.return_value = mock_existing()
         snapshot_base_manifest(self.pr_version, self.base_version)
-
-        for call in storage_open.call_args_list:
-            args, _ = call
-            if len(args) > 1:
-                assert args[1] != "w", "Should not write when snapshot exists"
+        storage_open.assert_not_called()