feat(git): Make ChangeSet a Struct wrapping a dict instead of a dict subclass

The composition pattern is safer than direct inheritance + using structs allows for easy serialization and deserialization, which is already useful for test data but will be even more in the future, in order to store build metadata.

Author: Ishirui

src/dda/tools/git.py

@@ -171,7 +171,7 @@ def get_working_tree_changes(self) -> ChangeSet:
         untracked_changes = ChangeSet.generate_from_diff_output(diffs)
 
         # Combine the changes
-        return ChangeSet(tracked_changes | untracked_changes)
+        return tracked_changes | untracked_changes
 
     def get_merge_base(self, remote_name: str = "origin") -> SHA1Hash | str:
         """

src/dda/utils/fs.py

@@ -152,6 +152,30 @@ def __as_exe(self) -> Path:
         def __as_exe(self) -> Path:
             return self
 
+    @classmethod
+    def enc_hook(cls, obj: Any) -> Any:
+        if isinstance(obj, cls):
+            return repr(obj)
+
+        message = f"Objects of type {type(obj)} are not supported"
+        raise NotImplementedError(message)
+
+    @classmethod
+    def dec_hook(cls, obj_type: type, obj: Any) -> Any:
+        if obj_type is cls:
+            # Was encoded as the repr of this object
+            # Should be of the form f"{cls.__qualname__}({str(obj)})"
+            qualname, path = obj.split("(")
+            path = path.rstrip(")").strip("'").strip('"')
+            if qualname != cls.__qualname__:
+                message = f"Objects of type {obj_type} are not supported"
+                raise NotImplementedError(message)
+
+            return cls(path)
+
+        message = f"Objects of type {obj_type} are not supported"
+        raise NotImplementedError(message)
+
 
 @contextmanager
 def temp_directory() -> Generator[Path, None, None]:

src/dda/utils/git/changeset.py

@@ -1,24 +1,30 @@
 # SPDX-FileCopyrightText: 2025-present Datadog, Inc. <dev@datadoghq.com>
 #
 # SPDX-License-Identifier: MIT
-from collections.abc import Generator
+
+from __future__ import annotations
+
 from enum import StrEnum
 from functools import cached_property
-from typing import Self
+from typing import TYPE_CHECKING, Any, Self
 
-from msgspec import Struct
+from msgspec import Struct, field
 
 from dda.utils.fs import Path
 from dda.utils.git.commit import SHA1Hash
 
+if TYPE_CHECKING:
+    from _collections_abc import dict_items, dict_keys, dict_values
+    from collections.abc import Generator, Iterable, Iterator
+
 
 class ChangeType(StrEnum):
     ADDED = "A"
     MODIFIED = "M"
     DELETED = "D"
 
     @classmethod
-    def from_github_status(cls, status: str) -> "ChangeType":
+    def from_github_status(cls, status: str) -> ChangeType:
         if status == "added":
             return cls.ADDED
         if status == "modified":
@@ -30,7 +36,7 @@ def from_github_status(cls, status: str) -> "ChangeType":
         raise ValueError(msg)
 
 
-class FileChanges(Struct):
+class FileChanges(Struct, frozen=True):
     """Represents changes to a single file in a git repository."""
 
     file: Path
@@ -164,16 +170,54 @@ def from_dict(cls, data: dict) -> Self:
             patch=data["patch"],
         )
 
+    @classmethod
+    def enc_hook(cls, obj: Any) -> Any:
+        # Only unsupported objects are Path objects
+        return Path.enc_hook(obj)
+
+    @classmethod
+    def dec_hook(cls, obj_type: type, obj: Any) -> Any:  # type: ignore[valid-type]
+        # Only unsupported objects are Path objects
+        return Path.dec_hook(obj_type, obj)
+
+
+# Need dict=True so that cached_property can be used
+class ChangeSet(Struct, dict=True, frozen=True):
+    _changes: dict[Path, FileChanges] = field(default_factory=dict)
 
-# Easier and safer to subclass UserDict than Dict directly
-class ChangeSet(dict[Path, FileChanges]):
     """
     Represents a set of changes to files in a git repository.
     This can both be a change between two commits, or the changes in the working directory.
 
     When considering the changes to the working directory, the untracked files are considered as added files.
     """
 
+    # == dict proxy methods == #
+    def keys(self) -> dict_keys[Path, FileChanges]:
+        return self._changes.keys()
+
+    def values(self) -> dict_values[Path, FileChanges]:
+        return self._changes.values()
+
+    def items(self) -> dict_items[Path, FileChanges]:
+        return self._changes.items()
+
+    def __getitem__(self, key: Path) -> FileChanges:
+        return self._changes[key]
+
+    def __contains__(self, key: Path) -> bool:
+        return key in self._changes
+
+    def __len__(self) -> int:
+        return len(self._changes)
+
+    def __iter__(self) -> Iterator[Path]:
+        return iter(self._changes.keys())
+
+    def __or__(self, other: Self) -> Self:
+        return self.from_iter(list(self.values()) + list(other.values()))
+
+    # == properties == #
     @cached_property
     def added(self) -> set[Path]:
         """List of files that were added."""
@@ -194,10 +238,7 @@ def changed(self) -> set[Path]:
         """List of files that were changed (added, modified, or deleted)."""
         return set(self.keys())
 
-    def add(self, change: FileChanges) -> None:
-        """Add a file change to the changeset."""
-        self[change.file] = change
-
+    # == methods == #
     def digest(self) -> SHA1Hash:
         """Compute a hash of the changeset."""
         from hashlib import sha1
@@ -210,21 +251,26 @@ def digest(self) -> SHA1Hash:
 
         return SHA1Hash(digester.hexdigest())
 
+    @classmethod
+    def from_iter(cls, data: Iterable[FileChanges]) -> Self:
+        """Create a ChangeSet from an iterable of FileChanges."""
+        items = {change.file: change for change in data}
+        return cls(_changes=items)
+
     @classmethod
     def generate_from_diff_output(cls, diff_output: str | list[str]) -> Self:
         """
         Generate a changeset from the output of a git diff command.
         The output should be passed as a string or a list of lines.
         """
-        changeset = cls()
-        for change in FileChanges.generate_from_diff_output(diff_output):
-            changeset.add(change)
-        return changeset
+        return cls.from_iter(FileChanges.generate_from_diff_output(diff_output))
+
+    @classmethod
+    def enc_hook(cls, obj: Any) -> Any:
+        # Only unsupported objects are Path objects
+        return Path.enc_hook(obj)
 
     @classmethod
-    def from_list(cls, data: list[dict]) -> Self:
-        """Create a ChangeSet from a JSON-serializable list."""
-        changeset = cls()
-        for change in data:
-            changeset.add(FileChanges.from_dict(change))
-        return changeset
+    def dec_hook(cls, obj_type: type, obj: Any) -> Any:
+        # Only unsupported objects are Path objects
+        return Path.dec_hook(obj_type, obj)

src/dda/utils/git/commit.py

@@ -73,15 +73,14 @@ def get_details_and_changes_from_github(self) -> tuple[CommitDetails, ChangeSet]
         data = client.get(self.github_api_url).json()
 
         # Compute ChangeSet
-        changes = ChangeSet()
-        for file_obj in data["files"]:
-            changes.add(
-                FileChanges(
-                    file=Path(file_obj["filename"]),
-                    type=ChangeType.from_github_status(file_obj["status"]),
-                    patch=file_obj["patch"],
-                )
+        changes = ChangeSet.from_iter(
+            FileChanges(
+                file=Path(file_obj["filename"]),
+                type=ChangeType.from_github_status(file_obj["status"]),
+                patch=file_obj["patch"],
             )
+            for file_obj in data["files"]
+        )
         self._changes = changes
 
         self._details = CommitDetails(

tests/tools/conftest.py

@@ -1,8 +1,8 @@
-import json
 import os
 import shutil
 from collections.abc import Callable, Generator
 
+import msgspec
 import pytest
 from _pytest.fixtures import SubRequest
 
@@ -157,8 +157,7 @@ def _make_repo_changes(
 
 def _load_changeset(filepath: Path) -> ChangeSet:
     with open(filepath, encoding="utf-8") as f:
-        data = json.load(f)
-    return ChangeSet.from_list(data)
+        return msgspec.json.decode(f.read(), type=ChangeSet, dec_hook=ChangeSet.dec_hook)
 
 
 @pytest.fixture(params=REPO_TESTCASES)

tests/tools/fixtures/repo_states/complex_changes/expected_changeset.json

@@ -1,27 +1 @@
-[
-    {
-        "file": "file2.txt",
-        "change_type": "removed",
-        "patch": "@@ -1,4 +0,0 @@\n-I am file2 !\n-I feel like I take up space for nothing...\n-I have a feeling like I won't exist pretty soon :/\n-"
-    },
-    {
-        "file": "file3.txt",
-        "change_type": "added",
-        "patch": "@@ -0,0 +1,2 @@\n+I am file3 !\n+I'm new around here, hopefully everyone treats me nice :)"
-    },
-    {
-        "file": "file4.txt",
-        "change_type": "modified",
-        "patch": "@@ -2 +2 @@ I am file4.\n-People often tell me I am unreliable.\n+People often tell me I am THE BEST.\n@@ -4,3 +4,2 @@ Things like:\n-- You always change !\n-- I can never count on you...\n-- I didn't recognize you !\n+- You rock !\n+- I wish I were you !\n@@ -8 +7,3 @@ Do you think they have a point ?\n-I'd need to look at my own history to know...\n+Pah ! Who am I kidding, they're OBVIOUSLY RIGHT.\n+Arrogance ? What is that, an italian ice cream flavor ?\n+Get outta here !"
-    },
-    {
-        "file": "file5.txt",
-        "change_type": "removed",
-        "patch": "@@ -1,5 +0,0 @@\n-I am a humble file.\n-Soon I will change name.\n-I think I'll also take this as an opportunity to change myself.\n-New name, new me !\n-Or is that not how the saying goes ?"
-    },
-    {
-        "file": "file5_new.txt",
-        "change_type": "added",
-        "patch": "@@ -0,0 +1,5 @@\n+I am a humble file.\n+Hey I have a new name !\n+Wow, I feel much better now.\n+New name, new me !\n+Or is that not how the saying goes ?"
-    }
-]
+{"_changes":{"Path('file2.txt')":{"file":"Path('file2.txt')","type":"D","patch":"@@ -1,4 +0,0 @@\n-I am file2 !\n-I feel like I take up space for nothing...\n-I have a feeling like I won't exist pretty soon :/\n-"},"Path('file3.txt')":{"file":"Path('file3.txt')","type":"A","patch":"@@ -0,0 +1,2 @@\n+I am file3 !\n+I'm new around here, hopefully everyone treats me nice :)"},"Path('file4.txt')":{"file":"Path('file4.txt')","type":"M","patch":"@@ -2 +2 @@ I am file4.\n-People often tell me I am unreliable.\n+People often tell me I am THE BEST.\n@@ -4,3 +4,2 @@ Things like:\n-- You always change !\n-- I can never count on you...\n-- I didn't recognize you !\n+- You rock !\n+- I wish I were you !\n@@ -8 +7,3 @@ Do you think they have a point ?\n-I'd need to look at my own history to know...\n+Pah ! Who am I kidding, they're OBVIOUSLY RIGHT.\n+Arrogance ? What is that, an italian ice cream flavor ?\n+Get outta here !"},"Path('file5.txt')":{"file":"Path('file5.txt')","type":"D","patch":"@@ -1,5 +0,0 @@\n-I am a humble file.\n-Soon I will change name.\n-I think I'll also take this as an opportunity to change myself.\n-New name, new me !\n-Or is that not how the saying goes ?"},"Path('file5_new.txt')":{"file":"Path('file5_new.txt')","type":"A","patch":"@@ -0,0 +1,5 @@\n+I am a humble file.\n+Hey I have a new name !\n+Wow, I feel much better now.\n+New name, new me !\n+Or is that not how the saying goes ?"}}}

tests/tools/fixtures/repo_states/file_add/expected_changeset.json

@@ -1,7 +1 @@
-[
-    {
-        "file": "file2.txt",
-        "change_type": "added",
-        "patch": "@@ -0,0 +1,3 @@\n+file2\n+I am a new file in the repo !\n+That's incredible."
-    }
-]
+{"_changes":{"Path('file2.txt')":{"file":"Path('file2.txt')","type":"A","patch":"@@ -0,0 +1,3 @@\n+file2\n+I am a new file in the repo !\n+That's incredible."}}}

tests/tools/fixtures/repo_states/file_delete/expected_changeset.json

@@ -1,7 +1 @@
-[
-    {
-        "file": "file2.txt",
-        "change_type": "removed",
-        "patch": "@@ -1,3 +0,0 @@\n-file2\n-I will be deleted, unfortunately.\n-That's quite sad."
-    }
-]
+{"_changes":{"Path('file2.txt')":{"file":"Path('file2.txt')","type":"D","patch":"@@ -1,3 +0,0 @@\n-file2\n-I will be deleted, unfortunately.\n-That's quite sad."}}}

tests/tools/fixtures/repo_states/file_edits/expected_changeset.json

@@ -1,27 +1 @@
-[
-    {
-        "file": "added_lines.txt",
-        "change_type": "modified",
-        "patch": "@@ -2,0 +3,4 @@ I wonder what that could mean ?\n+\n+But of course !\n+I get some added lines.\n+That makes sense."
-    },
-    {
-        "file": "added_lines_middle.txt",
-        "change_type": "modified",
-        "patch": "@@ -2,0 +3,2 @@ I have a bit more text than added_lines.\n+Nobody expects the Spanish Inquisition !\n+My developer really wonders if cracking jokes in test data is against company policy."
-    },
-    {
-        "file": "deleted_lines.txt",
-        "change_type": "modified",
-        "patch": "@@ -2,2 +2 @@ Hmm, my name is deleted_lines.\n-I wonder what that could mean ?\n-I don't want to be shortened, that does not seem fun.\n+"
-    },
-    {
-        "file": "deleted_lines_middle.txt",
-        "change_type": "modified",
-        "patch": "@@ -2,3 +1,0 @@ Hmm, my name is deleted_lines_middle.\n-I wonder what that could mean ?\n-I don't want to be shortened, that does not seem fun.\n-Thinking about it, being short is also kind of nice though."
-    },
-    {
-        "file": "edited_lines.txt",
-        "change_type": "modified",
-        "patch": "@@ -4,8 +4,5 @@ I have a big block of text here:\n-Lorem ipsum dolor sit amet, consectetur adipiscing elit.\n-Vestibulum ornare cursus diam, quis hendrerit nibh.\n-Donec sollicitudin neque in tempus ornare.\n-Integer sit amet pretium quam.\n-Maecenas lacinia augue id est malesuada, vitae fermentum justo faucibus.\n-Aenean posuere nisi tincidunt nisi pharetra blandit.\n-Integer sed nulla sed eros aliquet eleifend quis ac quam.\n-Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus.\n+What kind of dev uses lorem ipsum ?\n+Keyboard mashing is where it's at !\n+Check this out:\n+phq w4q3t'p\u00a3tfnjklifewqkpjnoq23bjikkijq4ovikobjqv4kioblj;\n+vqpewkvnkqknbpjlo[iqervkb[jplofqvwer[kpjlqfp[okqolp[f;vn]]]]]"
-    }
-]
+{"_changes":{"Path('added_lines.txt')":{"file":"Path('added_lines.txt')","type":"M","patch":"@@ -2,0 +3,4 @@ I wonder what that could mean ?\n+\n+But of course !\n+I get some added lines.\n+That makes sense."},"Path('added_lines_middle.txt')":{"file":"Path('added_lines_middle.txt')","type":"M","patch":"@@ -2,0 +3,2 @@ I have a bit more text than added_lines.\n+Nobody expects the Spanish Inquisition !\n+My developer really wonders if cracking jokes in test data is against company policy."},"Path('deleted_lines.txt')":{"file":"Path('deleted_lines.txt')","type":"M","patch":"@@ -2,2 +2 @@ Hmm, my name is deleted_lines.\n-I wonder what that could mean ?\n-I don't want to be shortened, that does not seem fun.\n+"},"Path('deleted_lines_middle.txt')":{"file":"Path('deleted_lines_middle.txt')","type":"M","patch":"@@ -2,3 +1,0 @@ Hmm, my name is deleted_lines_middle.\n-I wonder what that could mean ?\n-I don't want to be shortened, that does not seem fun.\n-Thinking about it, being short is also kind of nice though."},"Path('edited_lines.txt')":{"file":"Path('edited_lines.txt')","type":"M","patch":"@@ -4,8 +4,5 @@ I have a big block of text here:\n-Lorem ipsum dolor sit amet, consectetur adipiscing elit.\n-Vestibulum ornare cursus diam, quis hendrerit nibh.\n-Donec sollicitudin neque in tempus ornare.\n-Integer sit amet pretium quam.\n-Maecenas lacinia augue id est malesuada, vitae fermentum justo faucibus.\n-Aenean posuere nisi tincidunt nisi pharetra blandit.\n-Integer sed nulla sed eros aliquet eleifend quis ac quam.\n-Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus.\n+What kind of dev uses lorem ipsum ?\n+Keyboard mashing is where it's at !\n+Check this out:\n+phq w4q3t'p£tfnjklifewqkpjnoq23bjikkijq4ovikobjqv4kioblj;\n+vqpewkvnkqknbpjlo[iqervkb[jplofqvwer[kpjlqfp[okqolp[f;vn]]]]]"}}}

tests/tools/fixtures/repo_states/file_rename/expected_changeset.json

@@ -1,22 +1 @@
-[
-    {
-        "file": "file1.txt",
-        "change_type": "removed",
-        "patch": "@@ -1,3 +0,0 @@\n-I am a humble file.\n-Soon I will change name.\n-But that won't change who I am, my contents !"
-    },
-    {
-        "file": "file1_new.txt",
-        "change_type": "added",
-        "patch": "@@ -0,0 +1,3 @@\n+I am a humble file.\n+Soon I will change name.\n+But that won't change who I am, my contents !"
-    },
-    {
-        "file": "file2.txt",
-        "change_type": "removed",
-        "patch": "@@ -1,5 +0,0 @@\n-I am a humble file.\n-Soon I will change name.\n-I think I'll also take this as an opportunity to change myself.\n-New name, new me !\n-Or is that not how the saying goes ?"
-    },
-    {
-        "file": "file2_new.txt",
-        "change_type": "added",
-        "patch": "@@ -0,0 +1,5 @@\n+I am a humble file.\n+Hey I have a new name !\n+Wow, I feel much better now.\n+New name, new me !\n+Or is that not how the saying goes ?"
-    }
-]
+{"_changes":{"Path('file1.txt')":{"file":"Path('file1.txt')","type":"D","patch":"@@ -1,3 +0,0 @@\n-I am a humble file.\n-Soon I will change name.\n-But that won't change who I am, my contents !"},"Path('file1_new.txt')":{"file":"Path('file1_new.txt')","type":"A","patch":"@@ -0,0 +1,3 @@\n+I am a humble file.\n+Soon I will change name.\n+But that won't change who I am, my contents !"},"Path('file2.txt')":{"file":"Path('file2.txt')","type":"D","patch":"@@ -1,5 +0,0 @@\n-I am a humble file.\n-Soon I will change name.\n-I think I'll also take this as an opportunity to change myself.\n-New name, new me !\n-Or is that not how the saying goes ?"},"Path('file2_new.txt')":{"file":"Path('file2_new.txt')","type":"A","patch":"@@ -0,0 +1,5 @@\n+I am a humble file.\n+Hey I have a new name !\n+Wow, I feel much better now.\n+New name, new me !\n+Or is that not how the saying goes ?"}}}