Validate identity-preserving repairs to nested GitHub references (#666)

CONTRIBUTING.md

@@ -175,7 +175,13 @@ with supporting evidence in the PR description. A cleanup batch may change more
 entries or only remove entries. The reviewer identifies cleanup from the diff: every added
 entry line must correspond one-to-one to a removed entry for the same project. Unrelated new
 projects belong in a separate contribution PR; ordinary contributions retain the limit of
-five added entry lines. Non-entry prose or heading changes are outside cleanup scope.
+five added entry lines. Existing indented non-entry references may receive a canonical
+GitHub repository URL repair only: exactly one HTTPS repository link may change, the old and new links must
+resolve to the same immutable GitHub repository ID, and all surrounding text, indentation,
+category, parent entry occurrence, and relative position must remain unchanged. Pair each repair with its existing
+reference; new references, extra copies, removals, and other non-entry prose or heading
+changes are outside cleanup scope. Offline validation checks the repair structure;
+`PR Review` additionally verifies repository identity.
 
 Changed entries still undergo format, section, HTTPS, duplicate, primary-link reachability,
 and repository/documentation checks. Within the same section, cleanup does not reapply the

scripts/readme_entries.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import difflib
 import re
 from dataclasses import dataclass
 from pathlib import Path
@@ -129,3 +130,75 @@ def iter_readme_entries(path: str | Path) -> Iterable[ReadmeEntry]:
                 languages=languages,
                 description=description.strip(),
             )
+
+
+def nested_reference_repairs(base: str, head: str) -> list[tuple[int, int, str, str]]:
+    """Find existing nested references with only one repository URL changed.
+
+    Results contain zero-based old/new line indexes and URLs. This checks
+    structure only; the authenticated reviewer must verify repository identity.
+    """
+    before, after = base.splitlines(), head.splitlines()
+
+    def contexts(lines: list[str]) -> list[tuple[str, int | None]]:
+        section = ""
+        parent = None
+        result = []
+        for index, line in enumerate(lines):
+            if line.startswith("## "):
+                section, parent = line, None
+            elif line.startswith("- "):
+                parent = index if ENTRY_RE.match(line) else None
+            result.append((section, parent))
+        return result
+
+    old_context, new_context = contexts(before), contexts(after)
+    operations = difflib.SequenceMatcher(
+        a=before, b=after, autojunk=False
+    ).get_opcodes()
+    unchanged_lines = {
+        old: new
+        for operation, a, b, c, d in operations if operation == "equal"
+        for old, new in zip(range(a, b), range(c, d))
+    }
+    repairs = []
+    repository_url = re.compile(
+        r"https://github\.com/[A-Za-z0-9_-]+/[A-Za-z0-9_.-]+/?"
+    )
+    for operation, a, b, c, d in operations:
+        if operation != "replace" or b - a != d - c:
+            continue
+        for old_index, new_index in zip(range(a, b), range(c, d)):
+            old, new = before[old_index], after[new_index]
+            if not re.match(r"^[ \t]+- ", new) or ENTRY_RE.match(new):
+                continue
+            old_section, old_parent = old_context[old_index]
+            new_section, new_parent = new_context[new_index]
+            if (ENTRY_RE.match(old) or not old_section or old_section != new_section
+                    or old_parent is None or new_parent is None):
+                continue
+            # Anchor to the same unchanged parent occurrence, not merely its text.
+            # A URL repair cannot change the reference's position below that parent.
+            if (unchanged_lines.get(old_parent) != new_parent
+                    or old_index - old_parent != new_index - new_parent):
+                continue
+            old_links = list(MARKDOWN_URL_RE.finditer(old))
+            new_links = list(MARKDOWN_URL_RE.finditer(new))
+            if len(old_links) != len(new_links):
+                continue
+            changed = [
+                (a, b) for a, b in zip(old_links, new_links)
+                if a.group(1) != b.group(1)
+            ]
+            if len(changed) != 1:
+                continue
+            old_link, new_link = changed[0]
+            old_url, new_url = old_link.group(1), new_link.group(1)
+            if not all(repository_url.fullmatch(url) for url in (old_url, new_url)):
+                continue
+            if (old[:old_link.start(1)], old[old_link.end(1):]) != (
+                new[:new_link.start(1)], new[new_link.end(1):]
+            ):
+                continue
+            repairs.append((old_index, new_index, old_url, new_url))
+    return repairs

scripts/review_pr.py

@@ -12,6 +12,7 @@ import re
 import socket
 import ssl
 import sys
+from collections import Counter
 from dataclasses import dataclass
 from datetime import datetime, timedelta, timezone
 from functools import cache
@@ -33,6 +34,7 @@ from scripts.readme_entries import (
     MARKDOWN_URL_RE,
     VALID_SECTIONS,
     extract_languages,
+    nested_reference_repairs,
 )
 
 
@@ -780,8 +782,37 @@ def review_pr(
 
     base_readme = read_readme(repository, pull_request.base.sha)
     head_readme = read_readme(repository, pull_request.head.sha)
+    patch_text = files[0].patch
+    verified_lines: Counter[str] = Counter()
+    reference_repairs = nested_reference_repairs(base_readme, head_readme)
+    for old_index, new_index, old_url, new_url in reference_repairs:
+        old_id, new_id = repository_id(old_url), repository_id(new_url)
+        if old_id is None or old_id != new_id:
+            findings.append(Finding(
+                "reference", "nested reference must retain its verified GitHub repository identity"
+            ))
+            continue
+        verified_lines["-" + base_readme.splitlines()[old_index]] += 1
+        verified_lines["+" + head_readme.splitlines()[new_index]] += 1
+    if verified_lines:
+        remaining_patch = []
+        for line in (patch_text or "").splitlines():
+            if verified_lines[line]:
+                verified_lines[line] -= 1
+            else:
+                remaining_patch.append(line)
+        patch_text = "\n".join(remaining_patch)
+        if any(verified_lines.values()):
+            findings.append(Finding(
+                "reference", "nested reference repair missing from PR patch"
+            ))
+        elif not any(
+            line.startswith(("+", "-")) and not line.startswith(("+++", "---"))
+            for line in remaining_patch
+        ):
+            return findings, pull_request.title, 0
     entry_lines, removed_entry_lines, change_findings = analyze_readme_change(
-        files[0].patch, repository_id,
+        patch_text, repository_id,
     )
     findings.extend(change_findings)
     if not entry_lines:

scripts/validate_readme.py

@@ -32,6 +32,7 @@ from scripts.readme_entries import (
     ReadmeEntry,
     extract_languages,
     iter_readme_entries,
+    nested_reference_repairs,
 )
 
 
@@ -255,11 +256,15 @@ def validate_entry(
 
 
 def validate_malformed_added_lines(
-    readme_path: Path, added_lines: set[int]
+    readme_path: Path, added_lines: set[int], *, base_readme: str = ""
 ) -> list[Issue]:
     issues: list[Issue] = []
-    lines = readme_path.read_text(encoding="utf-8").splitlines()
-    for line_number in sorted(added_lines):
+    head_readme = readme_path.read_text(encoding="utf-8")
+    lines = head_readme.splitlines()
+    repaired_lines = {
+        new + 1 for _, new, _, _ in nested_reference_repairs(base_readme, head_readme)
+    }
+    for line_number in sorted(added_lines - repaired_lines):
         if line_number < 1 or line_number > len(lines):
             continue
         line = lines[line_number - 1]
@@ -315,10 +320,21 @@ def main() -> int:
     duplicate_names, duplicate_urls = build_duplicate_indexes(entries)
 
     added_lines: set[int] | None = None
+    base_readme = ""
     if args.diff_from:
         try:
             added_lines = read_added_line_numbers(args.diff_from, readme_path)
-        except RuntimeError as exc:
+            merge_base = subprocess.check_output(
+                ["git", "merge-base", args.diff_from, "HEAD"], text=True
+            ).strip()
+            root = subprocess.check_output(
+                ["git", "rev-parse", "--show-toplevel"], text=True
+            ).strip()
+            relative_path = readme_path.resolve().relative_to(Path(root))
+            base_readme = subprocess.check_output(
+                ["git", "show", f"{merge_base}:{relative_path}"], text=True
+            )
+        except (RuntimeError, subprocess.CalledProcessError, ValueError) as exc:
             print(f"ERROR {exc}", file=sys.stderr)
             return 2
         selected_entries = [
@@ -335,7 +351,9 @@ def main() -> int:
 
     issues: list[Issue] = []
     if added_lines is not None:
-        issues.extend(validate_malformed_added_lines(readme_path, added_lines))
+        issues.extend(validate_malformed_added_lines(
+            readme_path, added_lines, base_readme=base_readme
+        ))
 
     for entry in selected_entries:
         issues.extend(

tests/test_nested_reference_repairs.py

@@ -0,0 +1,130 @@
+import difflib
+import subprocess
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+from unittest.mock import patch
+
+from github import GithubException
+
+from scripts.readme_entries import iter_readme_entries, nested_reference_repairs
+from scripts.validate_readme import validate_malformed_added_lines
+from tests import test_review_pr as fixtures
+
+OLD = (
+    '  - Library - Python [package](https://pypi.org/project/demo/) '
+    'and [C++](https://github.com/example/old)'
+)
+NEW = OLD.replace('/old)', '/new)')
+PARENT = '- [Parent](https://github.com/example/parent) - `Python` - Toolkit.\n'
+BASE = '## Trading & Backtesting\n' + PARENT + OLD + '\n'
+
+
+class NestedReferenceRepairTests(unittest.TestCase):
+    def review(self, head, *, base=BASE, identity=123):
+        original = fixtures.FakeClient.get_repo
+        def lookup(client, name):
+            if identity is None and name == 'example/old':
+                raise GithubException(404, {'message': 'Not Found'})
+            repo = original(client, name)
+            if name.startswith('example/'):
+                repo.id = 123 if name.endswith('/old') else identity
+            return repo
+        patch_text = ''.join(difflib.unified_diff(
+            base.splitlines(keepends=True), head.splitlines(keepends=True)
+        ))
+        with patch.object(fixtures.FakeClient, 'get_repo', lookup):
+            return fixtures.ValidationPipelineTests().review(
+                patch_text=patch_text, base_readme=base, head_readme=head
+            )
+
+    def test_existing_reference_only_repair(self):
+        self.assertEqual(self.review(BASE.replace(OLD, NEW)), set())
+
+    def test_repair_mixed_with_entry_update(self):
+        base = BASE + PARENT.replace('[Parent]', '[Other]').replace('/parent)', '/other)')
+        head = base.replace(OLD, NEW).replace('[Other]', '[Renamed]')
+        self.assertEqual(self.review(head, base=base), set())
+
+    def test_new_changed_moved_or_duplicated_references_rejected(self):
+        cases = [
+            BASE + NEW + '\n',
+            BASE.replace(OLD, NEW + '\n' + NEW),
+            BASE.replace(OLD, NEW.replace('Library -', 'Different -')),
+            BASE.replace(OLD, NEW.replace('  -', '   -')),
+            BASE.replace(OLD, NEW.lstrip()),
+            BASE.replace(OLD, NEW.replace('[C++]', '[New label]')),
+            BASE.replace(OLD, NEW.replace('/project/demo/', '/project/other/')),
+            (BASE.replace(OLD, '') + '## Market Data & Data Sources\n'
+             + PARENT.replace('[Parent]', '[Moved]') + NEW + '\n'),
+            BASE.replace(OLD, '') + PARENT.replace('[Parent]', '[Other]') + NEW + '\n',
+            BASE.replace(OLD + '\n', ''),
+        ]
+        for head in cases:
+            with self.subTest(head=head):
+                self.assertTrue(self.review(head))
+
+    def test_reference_cannot_move_between_identical_parent_occurrences(self):
+        base = BASE + PARENT
+        head = BASE.replace(OLD + '\n', '') + PARENT + NEW + '\n'
+        self.assertEqual(nested_reference_repairs(base, head), [])
+
+    def test_reference_cannot_reorder_within_parent(self):
+        other = OLD.replace('Library -', 'Other -')
+        base = BASE + other + '\n'
+        head = BASE.replace(OLD, other) + NEW + '\n'
+        self.assertEqual(nested_reference_repairs(base, head), [])
+
+    def test_replacement_or_unverified_repository_rejected(self):
+        for identity in (456, None):
+            with self.subTest(identity=identity):
+                self.assertTrue(self.review(BASE.replace(OLD, NEW), identity=identity))
+
+    def test_non_repository_destinations_rejected(self):
+        for url in ('http://github.com/example/new', 'https://user@github.com/example/new',
+                    'https://other.example/example/new', 'https://github.com/example/new?q=x',
+                    'https://github.com/example/new#x', 'https://github.com/example/new/tree/main'):
+            with self.subTest(url=url):
+                self.assertTrue(self.review(BASE.replace('https://github.com/example/old', url)))
+
+    def test_offline_validation_preserves_entry_count(self):
+        with tempfile.TemporaryDirectory() as directory:
+            readme = Path(directory) / 'README.md'
+            readme.write_text(BASE.replace(OLD, NEW))
+            self.assertEqual(len(list(iter_readme_entries(readme))), 1)
+            self.assertEqual(validate_malformed_added_lines(readme, {3}, base_readme=BASE), [])
+            self.assertTrue(validate_malformed_added_lines(
+                readme, {3}, base_readme=BASE.replace(OLD, '')
+            ))
+
+    def test_offline_cli_uses_merge_base_for_existing_reference(self):
+        script = Path(__file__).resolve().parents[1] / 'scripts' / 'validate_readme.py'
+        with tempfile.TemporaryDirectory() as directory:
+            def git(*args):
+                return subprocess.run(['git', *args], cwd=directory, check=True,
+                                      capture_output=True, text=True).stdout.strip()
+            git('init', '-q')
+            git('config', 'user.email', 'test@example.com')
+            git('config', 'user.name', 'Test')
+            readme = Path(directory) / 'README.md'
+            readme.write_text(BASE)
+            git('add', 'README.md')
+            git('commit', '-qm', 'Base')
+            base = git('rev-parse', 'HEAD')
+            readme.write_text(BASE.replace(OLD, NEW))
+            git('commit', '-qam', 'Repair')
+            result = subprocess.run(
+                [sys.executable, str(script), '--diff-from', base], cwd=directory,
+                capture_output=True, text=True,
+            )
+            self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+            self.assertIn('Validated 0 entries', result.stdout)
+            readme.write_text(BASE.replace(OLD, NEW) + NEW + '\n')
+            git('commit', '-qam', 'Extra reference')
+            result = subprocess.run(
+                [sys.executable, str(script), '--diff-from', base], cwd=directory,
+                capture_output=True, text=True,
+            )
+            self.assertEqual(result.returncode, 1, result.stdout + result.stderr)
+            self.assertIn('added README bullet does not match entry regex', result.stdout)