From aa540f322b7a6cc411622c7d4b8de13dd08db50e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20Schm=C3=B6lder?= Date: Thu, 13 Aug 2026 09:19:08 +0200 Subject: [PATCH 1/2] Read the output log without checking out the main branch Reading the run history used to hard-reset the output repository and check out its main branch, so merely enumerating candidate result branches could discard uncommitted work. The log is now read from the main branch ref instead. Matching a case that declares no environment no longer loads the recorded environment either, since run_history only exists on the main branch and the comparison was a no-op regardless. --- cadetrdm/logging.py | 31 +++++++++++ cadetrdm/repositories.py | 20 +++++-- tests/test_read_only_loading.py | 99 +++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 5 deletions(-) create mode 100644 tests/test_read_only_loading.py diff --git a/cadetrdm/logging.py b/cadetrdm/logging.py index 45c2d99..b092368 100644 --- a/cadetrdm/logging.py +++ b/cadetrdm/logging.py @@ -76,6 +76,12 @@ def fulfils_environment(self, environment: Environment): Instance of Environment class, with requirements as key: value pairs. :return: """ + # Environment matching is opt-in. Without requirements to check against, the + # recorded environment is not read at all, so loading results does not depend + # on the run_history files being present in the working tree. + if environment is None: + return True + if self._environment is None: self._load_environment() @@ -137,6 +143,31 @@ def n_entries(self) -> int: """int: Number of results stored in the repository.""" return len(self.entries) + @classmethod + def from_string(cls, content: str, filepath=None): + """ + Create an OutputLog from the raw contents of a log.tsv file. + + Used to read the log out of a git ref without checking that ref out. The + filepath is not read from, but is retained so that LogEntry can resolve the + run_history files next to it. + + :param content: + Raw tab-separated contents of a log.tsv file. + :param filepath: + Optional path the contents belong to. + """ + instance = cls() + instance._filepath = filepath + + lines = [line.split("\t") for line in content.splitlines() if line] + if not lines: + return instance + + instance._entry_list = lines + instance.entries: dict[str, LogEntry] = instance._entries_from_entry_list(instance._entry_list) + return instance + @classmethod def from_list(cls, entry_list: list[list[str]]): instance = cls() diff --git a/cadetrdm/repositories.py b/cadetrdm/repositories.py index 8bb454d..7006eb3 100644 --- a/cadetrdm/repositories.py +++ b/cadetrdm/repositories.py @@ -1565,11 +1565,21 @@ def output_log_file_path(self): @property def output_log(self): - if self.has_uncomitted_changes: - self._reset_hard_to_head(force_entry=True) - if not self.active_branch == self.main_branch: - self.checkout(self.main_branch) - return OutputLog(filepath=self.output_log_file_path) + """ + OutputLog: The run history recorded on the main branch. + + Read directly from the main branch ref, so that inspecting the log neither + checks out that branch nor touches the working tree. Reading the log used to + discard uncommitted changes and check out the main branch, which made loading + results a destructive operation. + """ + try: + log_content = self._git.show(f"{self.main_branch}:log.tsv") + except git.GitCommandError: + # No log.tsv on the main branch yet, e.g. in a freshly initialized repo. + return OutputLog() + + return OutputLog.from_string(log_content, filepath=self.path / "log.tsv") def print_output_log(self): self.checkout(self.main_branch) diff --git a/tests/test_read_only_loading.py b/tests/test_read_only_loading.py new file mode 100644 index 0000000..eb291f1 --- /dev/null +++ b/tests/test_read_only_loading.py @@ -0,0 +1,99 @@ +"""Regression tests pinning that reading results does not mutate git state. + +Loading archived results must not move either repository. These tests snapshot the +observable git state of both repositories, perform a read, and assert the snapshot is +unchanged. +""" + +from pathlib import Path + +import pytest + +from cadetrdm import ProjectRepo, initialize_repo + + +def git_state(repo): + """Snapshot the observable git state of a repository. + + Covers everything a read operation must leave alone: the checked out commit and + branch, the set of local branches, and whether the working tree is dirty. + """ + return { + "commit": repo.current_commit_hash, + "branch": str(repo.active_branch), + "branches": sorted(head.name for head in repo._git_repo.heads), + "is_dirty": repo._git_repo.is_dirty(untracked_files=True), + } + + +@pytest.fixture +def repo_with_results(tmp_path): + """A project repo holding one recorded result, left on the result branch. + + Leaving the output repo on the result branch rather than on main is what makes the + mutation visible: a read that checks out main would change the active branch. + """ + path_to_repo = tmp_path / "project" + initialize_repo(path_to_repo, "results") + + repo = ProjectRepo(path_to_repo) + with repo.track_results(results_commit_message="Add result"): + (repo.output_path / "result.csv").write_text("1,2,3\n") + + assert str(repo.output_repo.active_branch) != repo.output_repo.main_branch + + return repo + + +def test_reading_output_log_leaves_output_repo_untouched(repo_with_results): + output_repo = repo_with_results.output_repo + state_before = git_state(output_repo) + + output_repo.output_log + + assert git_state(output_repo) == state_before + + +def test_reading_output_log_preserves_uncommitted_changes(repo_with_results): + output_repo = repo_with_results.output_repo + scratch_file = Path(output_repo.path) / "uncommitted.txt" + scratch_file.write_text("work in progress\n") + + output_repo.output_log + + assert scratch_file.exists(), "reading the log discarded uncommitted work" + assert scratch_file.read_text() == "work in progress\n" + + +def test_output_log_read_without_checkout_lists_the_result_branch(repo_with_results): + output_repo = repo_with_results.output_repo + result_branch = str(output_repo.active_branch) + + entries = output_repo.output_log.entries + + assert result_branch in entries + assert entries[result_branch].project_repo_commit_hash == repo_with_results.current_commit_hash + + +def test_matching_without_an_environment_does_not_read_run_history(repo_with_results): + """Without requirements to match, recorded environments must not be read. + + run_history is only populated on the main branch, so on a result branch the + conda_environment.yml a LogEntry would load is not in the working tree. Reading it + used to work only because reading the log checked out main first. + """ + output_repo = repo_with_results.output_repo + assert not (Path(output_repo.path) / "run_history").exists() + + entry = output_repo.output_log.entries[str(output_repo.active_branch)] + + assert entry.fulfils_environment(None) is True + + +def test_output_log_is_empty_when_no_results_are_recorded(tmp_path): + path_to_repo = tmp_path / "project" + initialize_repo(path_to_repo, "results") + + repo = ProjectRepo(path_to_repo) + + assert repo.output_repo.output_log.n_entries == 0 From fee07b7772b59182e290bba75f07bdf781a57c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20Schm=C3=B6lder?= Date: Thu, 13 Aug 2026 10:44:28 +0200 Subject: [PATCH 2/2] Document output log write path --- cadetrdm/repositories.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cadetrdm/repositories.py b/cadetrdm/repositories.py index 7006eb3..fbdc07d 100644 --- a/cadetrdm/repositories.py +++ b/cadetrdm/repositories.py @@ -1095,6 +1095,9 @@ def update_output_main_logs( Dumps all the metadata information about the project repositories state and the commit hash and branch name of the ouput repository into the main branch of the output repository. + This is the write path for the output log: it intentionally checks out the + output repository's main branch, updates the run history, commits it, and then + returns to the result branch. :param output_dict: Dictionary containing key-value pairs to be added to the log. """