Address codex reviews

Author: sarahchen6

.github/scripts/dependency_age.py

@@ -121,7 +121,7 @@ def emit_outputs(outputs: dict[str, Any], github_output: str | None) -> None:
 def load_json(file_path: str | None, url: str | None) -> Any:
     if file_path:
         text = Path(file_path).read_text(encoding="utf-8")
-        text = re.sub(r"[ \t]*//[^\n]*", "", text)  # strip // line comments
+        text = re.sub(r"(?<!:)//[^\n]*", "", text)  # strip // line comments, preserve ://
         return json.loads(text)
     if not url:
         raise ValueError("either file_path or url is required")
@@ -295,7 +295,7 @@ def validate_lockfiles(args: argparse.Namespace) -> int:
     for relative_path, gav in changed:
         gav_to_files.setdefault(gav, set()).add(relative_path)
 
-    reverted: list[tuple[str, list[str], str]] = []
+    violations: list[tuple[str, list[str], str]] = []
     for gav in sorted(gav_to_files):
         published_at, reason = resolve_gav_timestamp(
             gav=gav,
@@ -304,10 +304,10 @@ def validate_lockfiles(args: argparse.Namespace) -> int:
         )
         affected_files = sorted(gav_to_files[gav])
         if published_at is None:
-            reverted.append((gav, affected_files, reason or "Unable to determine publish timestamp."))
+            violations.append((gav, affected_files, reason or "Unable to determine publish timestamp."))
             continue
         if published_at > cutoff:
-            reverted.append(
+            violations.append(
                 (
                     gav,
                     affected_files,
@@ -323,21 +323,21 @@ def validate_lockfiles(args: argparse.Namespace) -> int:
             f"(published {format_datetime(published_at)}, cutoff {format_datetime(cutoff)})"
         )
 
-    if reverted:
-        outputs["reverted_coordinates"] = len(reverted)
+    if violations:
+        outputs["reverted_coordinates"] = len(violations)
         revert_violations_in_lockfiles(
-            violations=reverted,
+            violations=violations,
             baseline_dir=baseline_dir,
             current_dir=current_dir,
         )
-        for gav, affected_files, message in reverted:
+        for gav, affected_files, message in violations:
             for path in affected_files:
                 print(f"::warning file={path}::{gav}: {message} Reverted to prior version.")
 
     emit_outputs(outputs, args.github_output)
     print(
         f"Validated {len(changed)} changed dependency selections against cutoff {format_datetime(cutoff)}. "
-        f"{len(reverted)} reverted."
+        f"{len(violations)} reverted."
     )
     return 0
 
@@ -363,16 +363,26 @@ def revert_violations_in_lockfiles(
         baseline_by_gav = read_lockfile_lines(baseline_path) if baseline_path.exists() else {}
         current_by_gav = read_lockfile_lines(current_path)
 
-        # For each violated GAV, find which baseline coordinate(s) it replaced: those
-        # that share the same group:artifact but are absent from the current lockfile.
-        predecessors_by_violated: dict[str, list[str]] = {}
-        for violated_gav in violated_gavs:
-            ga = ":".join(violated_gav.split(":")[:2])
-            removed = sorted(
-                b for b in baseline_by_gav
-                if ":".join(b.split(":")[:2]) == ga and b not in current_by_gav
-            )
-            predecessors_by_violated[violated_gav] = removed
+        # Group removed baseline GAVs and violated GAVs by group:artifact.
+        removed_by_ga: dict[str, list[str]] = {}
+        for b in sorted(baseline_by_gav):
+            if b not in current_by_gav:
+                ga = ":".join(b.split(":")[:2])
+                removed_by_ga.setdefault(ga, []).append(b)
+
+        violations_by_ga: dict[str, list[str]] = {}
+        for v in sorted(violated_gavs):
+            ga = ":".join(v.split(":")[:2])
+            violations_by_ga.setdefault(ga, []).append(v)
+
+        # Pair each removed predecessor with the violation at the same sorted position.
+        # Excess predecessors (consolidation) pile onto the last violation.
+        # Violations with no corresponding predecessor are brand-new dependencies.
+        predecessors_by_violated: dict[str, list[str]] = {v: [] for v in violated_gavs}
+        for ga, ga_violations in violations_by_ga.items():
+            for i, pred in enumerate(removed_by_ga.get(ga, [])):
+                target = ga_violations[min(i, len(ga_violations) - 1)]
+                predecessors_by_violated[target].append(pred)
 
         output_lines: list[str] = []
         for raw_line in current_path.read_text(encoding="utf-8").splitlines():

.github/scripts/tests/fixtures/maven-prerelease-filter.json

@@ -11,7 +11,9 @@
 SCRIPT = REPO_ROOT / ".github/scripts/dependency_age.py"
 FIXTURES = Path(__file__).resolve().parent / "fixtures"
 NOW = "2026-04-24T12:00:00Z"
-OUTPUT_PATTERN = re.compile(r"^(cutoff_at|found|version|published_at|reason)=(.*)$")
+OUTPUT_PATTERN = re.compile(
+    r"^(cutoff_at|found|version|published_at|reason|validated_coordinates|reverted_coordinates)=(.*)$"
+)
 
 
 class DependencyAgeScriptTest(unittest.TestCase):
@@ -61,29 +63,6 @@ def test_reports_when_no_eligible_gradle_release_exists(self) -> None:
         self.assertEqual(outputs["found"], "false")
         self.assertIn("No eligible stable Gradle release", outputs["reason"])
 
-    def test_maven_prerelease_filtering_still_excludes_alpha_beta_and_rc(self) -> None:
-        result = self.run_script(
-            "select-maven",
-            "--now",
-            NOW,
-            "--group-id",
-            "org.apache.maven",
-            "--artifact-id",
-            "apache-maven",
-            "--search-response-file",
-            str(FIXTURES / "maven-prerelease-filter.json"),
-            "--prerelease-pattern",
-            "alpha",
-            "--prerelease-pattern",
-            "beta",
-            "--prerelease-pattern",
-            "rc",
-        )
-
-        self.assertEqual(result.returncode, 0, result.stderr)
-        outputs = self.parse_outputs(result.stdout)
-        self.assertEqual(outputs["version"], "3.9.9")
-
     def test_selects_previous_maven_release_when_newest_is_too_new(self) -> None:
         result = self.run_script(
             "select-maven",
@@ -129,7 +108,6 @@ def test_exact_48_hour_boundary_is_accepted(self) -> None:
         self.assertEqual(outputs["version"], "3.5.5")
         self.assertEqual(outputs["published_at"], "2026-04-22T12:00:00Z")
 
-
     def run_validate_lockfiles(
         self,
         *,
@@ -172,6 +150,38 @@ def run_validate_lockfiles(
         )
         return result, current_dir
 
+    def test_validates_changed_lockfiles_when_all_updates_are_old_enough(self) -> None:
+        lockfile = "module/gradle.lockfile"
+        baseline_content = "\n".join([
+            "# Gradle lockfile",
+            "com.example:lib-a:1.0.0=runtimeClasspath",
+            "com.example:lib-b:1.0.0=runtimeClasspath",
+            "",
+        ])
+        current_content = "\n".join([
+            "# Gradle lockfile",
+            "com.example:lib-a:1.1.0=runtimeClasspath",  # valid upgrade
+            "com.example:lib-b:1.1.0=runtimeClasspath",  # valid upgrade
+            "",
+        ])
+        metadata = {
+            "com.example:lib-a:1.1.0": "2026-04-20T12:00:00Z",  # old enough
+            "com.example:lib-b:1.1.0": "2026-04-20T11:00:00Z",  # old enough
+        }
+
+        result, current_dir = self.run_validate_lockfiles(
+            baseline={"module/gradle.lockfile": baseline_content},
+            current={"module/gradle.lockfile": current_content},
+            metadata=metadata,
+        )
+
+        self.assertEqual(result.returncode, 0, result.stderr)
+        outputs = self.parse_outputs(result.stdout)
+        self.assertEqual(outputs["validated_coordinates"], "2")
+        self.assertEqual(outputs["reverted_coordinates"], "0")
+        final = (current_dir / lockfile).read_text(encoding="utf-8")
+        self.assertEqual(final, current_content)
+
     def test_reverts_too_new_version_upgrade_keeps_valid_changes(self) -> None:
         lockfile = "module/gradle.lockfile"
         baseline_content = "\n".join([
@@ -236,6 +246,42 @@ def test_reverts_correct_version_when_multiple_versions_coexist(self) -> None:
         self.assertIn("com.typesafe:config:1.4.4=runtimeClasspath,testRuntimeClasspath", final)
         self.assertNotIn("com.typesafe:config:1.5.0", final)
 
+    def test_reverts_each_version_independently_for_same_group_artifact(self) -> None:
+        # Both versions of the same group:artifact are upgraded to too-new versions.
+        # Each must be reverted to its own predecessor, not both to the same line.
+        lockfile = "module/gradle.lockfile"
+        baseline_content = "\n".join([
+            "# Gradle lockfile",
+            "com.example:lib:1.0.0=compileClasspath",
+            "com.example:lib:2.0.0=runtimeClasspath",
+            "",
+        ])
+        current_content = "\n".join([
+            "# Gradle lockfile",
+            "com.example:lib:1.1.0=compileClasspath",  # too new, should revert to 1.0.0
+            "com.example:lib:2.1.0=runtimeClasspath",  # too new, should revert to 2.0.0
+            "",
+        ])
+        metadata = {
+            "com.example:lib:1.1.0": "2026-04-24T11:00:00Z",  # too new
+            "com.example:lib:2.1.0": "2026-04-24T11:00:00Z",  # too new
+        }
+
+        result, current_dir = self.run_validate_lockfiles(
+            baseline={"module/gradle.lockfile": baseline_content},
+            current={"module/gradle.lockfile": current_content},
+            metadata=metadata,
+        )
+
+        self.assertEqual(result.returncode, 0, result.stderr)
+        final = (current_dir / lockfile).read_text(encoding="utf-8")
+        self.assertIn("com.example:lib:1.0.0=compileClasspath", final)
+        self.assertIn("com.example:lib:2.0.0=runtimeClasspath", final)
+        self.assertNotIn("com.example:lib:1.1.0", final)
+        self.assertNotIn("com.example:lib:2.1.0", final)
+        # Exactly two entries — no duplication
+        self.assertEqual(final.count("com.example:lib:"), 2)
+
     def test_removes_brand_new_dependency_that_is_too_new(self) -> None:
         lockfile = "module/gradle.lockfile"
         baseline_content = "\n".join([

.github/scripts/tests/test_dependency_age.py

@@ -116,7 +116,7 @@ jobs:
           # What Does This Do
 
           This PR updates the Gradle dependency locks for common and product modules.
-          Changed dependencies were validated to be at least ${{ env.MIN_DEPENDENCY_AGE_HOURS }} hours old.
+          Dependency resolution was performed through delayed proxies, and changed dependencies were validated to be at least ${{ env.MIN_DEPENDENCY_AGE_HOURS }} hours old.
 
           # Motivation
 
@@ -177,7 +177,7 @@ jobs:
           # What Does This Do
 
           This PR updates the Gradle dependency locks for instrumentations and their tests.
-          Changed dependencies were validated to be at least ${{ env.MIN_DEPENDENCY_AGE_HOURS }} hours old.
+          Dependency resolution was performed through delayed proxies, and changed dependencies were validated to be at least ${{ env.MIN_DEPENDENCY_AGE_HOURS }} hours old.
 
           # Motivation