Add YamlDocument.delete(), remove stale version keys in freeze, complete test assertions

- yaml.py: add delete(jsonpath, field) — mirrors set() but removes the
  field and its child block; _delete_by_parts stops at blank lines to
  preserve inter-item separators; 5 unit tests added in test_yaml.py
- manifest.py update_project_version: call _doc.delete() for each
  version key that is now empty/absent (removes stale 'revision', 'tag',
  'branch') and remove the entire 'integrity' block when no hash is
  present; regression test added in test_manifest.py
- test_add.py: add append_project_entry.assert_called_once() +
  update_dump.assert_called_once() to the 5 tests that previously only
  accessed call_args without asserting call count:
  suffixes_duplicate_name, interactive_branch_by_number,
  svn_custom_branch, svn_tag, svn_branch_by_number

https://claude.ai/code/session_01Xd8EcAUkSoJo9YZimGzEuA

Author: claude

dfetch/manifest/manifest.py

@@ -418,9 +418,16 @@ def update_project_version(self, project: ProjectEntry) -> None:
                     f"Updating {project.name} version field '{name}' to '{value}' in manifest"
                 )
                 self._doc.set(path, name, value)
+            else:
+                # Remove any previously-pinned key that is no longer active
+                # (e.g. an old 'revision' when the project is now pinned by tag).
+                self._doc.delete(path, name)
 
         if project.integrity and project.integrity.hash:
             self._doc.set(path, "integrity.hash", project.integrity.hash)
+        else:
+            # Remove a stale integrity block if the project no longer carries one.
+            self._doc.delete(path, "integrity")
 
         self.__text = self._doc.dump()
 

dfetch/util/yaml.py

@@ -172,6 +172,60 @@ def set(self, jsonpath: str, field: str, value: str) -> None:
             )
             self._set_by_parts(target_parts, value)
 
+    def delete(self, jsonpath: str, field: str) -> None:
+        """Remove *field* from every node matched by *jsonpath*.
+
+        *field* may be a simple key (``"revision"``) or a top-level nested
+        key (``"integrity"``); all child lines are removed together with the
+        key itself.  If the field is absent the call is a no-op.
+
+        Example::
+
+            doc.delete(
+                '$.manifest.projects[?(@.name == "my-project")]',
+                "revision",
+            )
+        """
+        steps = self._parse_jsonpath(jsonpath)
+
+        filter_idx = next(
+            (i for i, s in enumerate(steps) if isinstance(s, _FilterStep)), None
+        )
+
+        if filter_idx is None:
+            parts = [s for s in steps if isinstance(s, str)] + [field]
+            self._delete_by_parts(parts)
+            return
+
+        sequence_parts = [s for s in steps[:filter_idx] if isinstance(s, str)]
+        filter_step: _FilterStep = steps[filter_idx]  # type: ignore[assignment]
+        post_filter_parts = [s for s in steps[filter_idx + 1 :] if isinstance(s, str)]
+
+        root = yaml.compose("".join(self.lines))
+        sequence_node = self._traverse_path(root, sequence_parts)
+        if not isinstance(sequence_node, SequenceNode):
+            return
+
+        # Collect targets first then delete in reverse so earlier deletions
+        # don't shift the line indices of later ones.
+        targets: list[list[str]] = []
+        for item_idx, item in enumerate(sequence_node.value):
+            if not isinstance(item, MappingNode):
+                continue
+            mapping = self._mapping_to_dict(item)
+            filter_node = mapping.get(filter_step.key)
+            if not (
+                isinstance(filter_node, ScalarNode)
+                and str(filter_node.value) == filter_step.value
+            ):
+                continue
+            targets.append(
+                sequence_parts + [str(item_idx)] + post_filter_parts + [field]
+            )
+
+        for parts in reversed(targets):
+            self._delete_by_parts(parts)
+
     def dump(self) -> str:
         """Return the current document as a string."""
         return "".join(self.lines)
@@ -270,6 +324,31 @@ def _eval_steps(
     # Text-level mutation helpers                                          #
     # ------------------------------------------------------------------ #
 
+    def _delete_by_parts(self, parts: list[str]) -> None:
+        """Remove the field at *parts* and all of its child lines.
+
+        Blank lines are treated as block separators and are never deleted,
+        so the surrounding layout is preserved even when a field is the last
+        item inside a sequence element.
+        """
+        idx = self._find_field(parts)
+        if idx is None:
+            return
+        line = self.lines[idx]
+        field_indent = len(line) - len(line.lstrip())
+        end_idx = idx + 1
+        while end_idx < len(self.lines):
+            next_stripped = self.lines[end_idx].rstrip("\n\r")
+            if not next_stripped:
+                break  # blank line ends this field's block
+            if next_stripped.lstrip().startswith("#"):
+                end_idx += 1
+                continue
+            if len(next_stripped) - len(next_stripped.lstrip()) <= field_indent:
+                break
+            end_idx += 1
+        del self.lines[idx:end_idx]
+
     def _set_by_parts(self, parts: list[str], value: str) -> None:
         """Locate the field at *parts* and update it, or add it if absent."""
         idx = self._find_field(parts)

tests/test_add.py

@@ -236,6 +236,8 @@ def test_add_command_suffixes_duplicate_name():
         ):
             Add()(_make_args("https://github.com/org/myrepo.git"))
 
+    fake_superproject.manifest.append_project_entry.assert_called_once()
+    fake_superproject.manifest.update_dump.assert_called_once()
     entry: ProjectEntry = fake_superproject.manifest.append_project_entry.call_args[0][0]
     assert entry.name == "myrepo-1"
 
@@ -317,6 +319,7 @@ def test_add_command_interactive_branch_by_number():
                         )
                     )
 
+    fake_superproject.manifest.update_dump.assert_called_once()
     entry: ProjectEntry = fake_superproject.manifest.append_project_entry.call_args[0][0]
     assert entry.branch == "dev"
 
@@ -677,6 +680,7 @@ def test_add_command_interactive_svn_custom_branch():
                 ):
                     Add()(_make_args(_SVN_URL, interactive=True))
 
+    fake_superproject.manifest.update_dump.assert_called_once()
     entry: ProjectEntry = fake_superproject.manifest.append_project_entry.call_args[0][0]
     assert entry.branch == "feature-x"
 
@@ -708,6 +712,7 @@ def test_add_command_interactive_svn_tag():
                 ):
                     Add()(_make_args(_SVN_URL, interactive=True))
 
+    fake_superproject.manifest.update_dump.assert_called_once()
     entry: ProjectEntry = fake_superproject.manifest.append_project_entry.call_args[0][0]
     assert entry.tag == "v2.0"
     assert entry.branch == ""
@@ -740,6 +745,7 @@ def test_add_command_interactive_svn_branch_by_number():
                 ):
                     Add()(_make_args(_SVN_URL, interactive=True))
 
+    fake_superproject.manifest.update_dump.assert_called_once()
     entry: ProjectEntry = fake_superproject.manifest.append_project_entry.call_args[0][0]
     assert entry.branch == "feature-x"
 

tests/test_manifest.py

@@ -371,3 +371,22 @@ def test_update_commented_out_field_is_appended_not_matched() -> None:
     # The live branch line keeps its value
     assert "      branch: main" in result
     assert "revision:" in result
+
+
+def test_update_removes_stale_revision_when_pinned_by_tag() -> None:
+    """update_project_version must delete stale 'revision' when the project is now pinned by tag."""
+    text = (
+        "manifest:\n"
+        "  version: '0.0'\n"
+        "  projects:\n"
+        "    - name: myproject\n"
+        "      url: https://example.com\n"
+        "      revision: oldrev\n"
+        "      branch: main\n"
+    )
+    # Project is now pinned exclusively by tag (revision and branch are empty).
+    project = _make_project("myproject", tag="v1.2.3")
+    result = _update(text, project)
+    assert "tag: v1.2.3" in result
+    assert "revision:" not in result
+    assert "branch:" not in result

tests/test_yaml.py

@@ -313,3 +313,62 @@ def test_get_field_present_in_second_project():
     matches = doc.get('$.manifest.projects[?(@.name == "second")].revision')
     assert len(matches) == 1
     assert matches[0].value == "abc123"
+
+
+# ---------------------------------------------------------------------------
+# delete()
+# ---------------------------------------------------------------------------
+
+
+def test_delete_removes_existing_scalar_field():
+    doc = YamlDocument(_MANIFEST)
+    doc.delete('$.manifest.projects[?(@.name == "myproject")]', "branch")
+    result = doc.dump()
+    # The myproject block should no longer contain branch
+    myproject_end = result.index("- name: other")
+    assert "branch:" not in result[:myproject_end]
+    # Sibling project and other fields are untouched
+    assert "url:" in result
+    assert "branch: develop" in result
+
+
+def test_delete_noop_when_field_absent():
+    doc = YamlDocument(_MANIFEST)
+    original = doc.dump()
+    doc.delete('$.manifest.projects[?(@.name == "myproject")]', "revision")
+    assert doc.dump() == original
+
+
+def test_delete_noop_when_path_no_match():
+    doc = YamlDocument(_MANIFEST)
+    original = doc.dump()
+    doc.delete('$.manifest.projects[?(@.name == "ghost")]', "branch")
+    assert doc.dump() == original
+
+
+def test_delete_removes_nested_block():
+    manifest = """\
+manifest:
+  version: '0.0'
+  projects:
+    - name: pkg
+      url: https://example.com
+      vcs: archive
+      integrity:
+        hash: sha256:abc123
+"""
+    doc = YamlDocument(manifest)
+    doc.delete('$.manifest.projects[?(@.name == "pkg")]', "integrity")
+    result = doc.dump()
+    assert "integrity:" not in result
+    assert "hash:" not in result
+    assert "url:" in result
+
+
+def test_delete_only_affects_matched_project():
+    doc = YamlDocument(_TWO_PROJECT_MANIFEST)
+    doc.delete('$.manifest.projects[?(@.name == "second")]', "revision")
+    result = doc.dump()
+    first_end = result.index("- name: second")
+    assert "branch: main" in result[:first_end]
+    assert "revision" not in result[first_end:]