ci: skip a Python member's quality matrix when a lock change doesn't touch it

.github/workflows/_quality-python.yml

@@ -44,8 +44,62 @@ permissions:
   contents: read
 
 jobs:
+  # The caller's path filter has to list uv.lock and the root pyproject.toml,
+  # so any lock change fans out to every Python member. Most lock changes touch
+  # one member's dependencies; this skips the matrix for the others. See
+  # platform/ci/quality_needed.py for the exact rule. It fails open: anything
+  # it cannot determine runs the matrix.
+  detect:
+    name: needs quality?
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    outputs:
+      run: ${{ steps.decide.outputs.run }}
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          # Pull requests check out the merge commit, whose first parent is
+          # the base branch tip, so HEAD^1..HEAD is exactly the PR's change.
+          fetch-depth: 2
+          persist-credentials: false
+
+      - uses: astral-sh/setup-uv@v5
+
+      - name: Decide
+        id: decide
+        shell: bash
+        env:
+          EVENT: ${{ github.event_name }}
+          BEFORE: ${{ github.event.before }}
+          # In a called workflow, workflow_ref names the caller:
+          # owner/repo/.github/workflows/<file>.yml@ref
+          WORKFLOW_REF: ${{ github.workflow_ref }}
+          MEMBER_PATH: ${{ inputs.path }}
+          SERVER: ${{ github.server_url }}
+          REPO: ${{ github.repository }}
+        run: |
+          set -euo pipefail
+          workflow="${WORKFLOW_REF#"$REPO"/}"
+          workflow="${workflow%@*}"
+          base=""
+          case "$EVENT" in
+            pull_request) base="$(git rev-parse HEAD^1)" ;;
+            push)
+              if [ -n "$BEFORE" ] && [ "$BEFORE" != "0000000000000000000000000000000000000000" ] \
+                 && git fetch --no-tags --depth=1 "$SERVER/$REPO" "$BEFORE" 2>/dev/null; then
+                base="$BEFORE"
+              fi
+              ;;
+          esac
+          uv run --no-project --with pyyaml python platform/ci/quality_needed.py \
+            --workflow "$workflow" --member-path "$MEMBER_PATH" --base "$base" \
+            | tee -a "$GITHUB_OUTPUT"
+          grep '^reason=' "$GITHUB_OUTPUT" | tail -1 | sed 's/^reason=/### Quality matrix: /' >> "$GITHUB_STEP_SUMMARY"
+
   quality:
     name: py${{ matrix.python }} · ${{ matrix.os }}
+    needs: detect
+    if: ${{ needs.detect.outputs.run == 'true' }}
     runs-on: ${{ matrix.os }}
     timeout-minutes: ${{ inputs.timeout-minutes }}
     strategy:

platform/ci/quality_needed.py

@@ -0,0 +1,166 @@
+"""Decide whether a Python member's quality matrix needs to run for this change.
+
+Called by ``_quality-python.yml``'s ``detect`` job. A member's ``paths:`` filter
+deliberately lists ``uv.lock``, because the workspace shares one lockfile and a
+change to it *can* change what the member installs. It usually does not: most
+lock changes add or bump another member's dependencies. Running algosystem's
+six-job OS/Python matrix for a data-ngin-only bump is the waste this removes.
+
+The rule, applied to the files changed between ``--base`` and ``--head``:
+
+* any changed file that matches the caller's ``pull_request.paths`` *other
+  than* ``uv.lock`` -> run (member code, its workspace deps, root config, CI)
+* only ``uv.lock`` matched -> run only if the member's resolved dependency
+  closure differs between the two locks. The closure is every package reachable
+  from the member (all extras and groups, which is what CI syncs) plus the
+  workspace root's dev group (pytest, ruff, mypy: the tools CI runs it with),
+  compared entry by entry, versions, sources and hashes included.
+* nothing matched -> skip (only reachable from push/dispatch, where GitHub has
+  already applied the path filter coarsely)
+
+Anything it cannot determine (no base commit, unreadable lock) means run: a
+skipped check that should have run is the expensive mistake here.
+
+Prints ``run=true|false`` and ``reason=...`` lines for ``$GITHUB_OUTPUT``.
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import subprocess
+import sys
+from collections import deque
+from pathlib import Path
+from typing import Any
+
+from _common import REPO_ROOT, discover_members, load_quality_workflows, path_matches
+
+try:  # 3.11+
+    import tomllib
+except ModuleNotFoundError:  # pragma: no cover - only on 3.9/3.10
+    import tomli as tomllib  # type: ignore[import-not-found,no-redef]
+
+LOCKFILE = "uv.lock"
+WORKSPACE_ROOT_SOURCE = {"virtual": "."}
+
+
+def _git(*args: str, root: Path = REPO_ROOT) -> str | None:
+    proc = subprocess.run(["git", *args], cwd=root, capture_output=True, text=True, check=False)
+    return proc.stdout if proc.returncode == 0 else None
+
+
+def lock_at(rev: str, root: Path = REPO_ROOT) -> dict[str, Any] | None:
+    text = _git("show", f"{rev}:{LOCKFILE}", root=root)
+    if text is None:
+        return None
+    try:
+        return tomllib.loads(text)
+    except tomllib.TOMLDecodeError:
+        return None
+
+
+def _edges(package: dict[str, Any]) -> list[str]:
+    deps = list(package.get("dependencies") or [])
+    for group in (package.get("optional-dependencies") or {}).values():
+        deps.extend(group)
+    for group in (package.get("dev-dependencies") or {}).values():
+        deps.extend(group)
+    return [d["name"] for d in deps]
+
+
+def closure(lock: dict[str, Any], member_name: str) -> dict[str, list[str]]:
+    """Every lock entry the member's environment can contain, as canonical JSON.
+
+    A name can appear more than once in a lock (per-platform forks), so each
+    name maps to all of its entries, sorted.
+    """
+    by_name: dict[str, list[dict[str, Any]]] = {}
+    for package in lock.get("package") or []:
+        by_name.setdefault(package["name"], []).append(package)
+
+    roots = [member_name]
+    roots += [
+        p["name"] for p in lock.get("package") or [] if p.get("source") == WORKSPACE_ROOT_SOURCE
+    ]
+
+    seen: dict[str, list[str]] = {}
+    queue = deque(roots)
+    while queue:
+        name = queue.popleft()
+        if name in seen or name not in by_name:
+            continue
+        entries = by_name[name]
+        seen[name] = sorted(json.dumps(e, sort_keys=True) for e in entries)
+        for entry in entries:
+            queue.extend(_edges(entry))
+    return seen
+
+
+def decide(
+    changed: list[str],
+    patterns: list[str],
+    member_name: str,
+    base: str,
+    head: str,
+    root: Path = REPO_ROOT,
+) -> tuple[bool, str]:
+    relevant = [f for f in changed if path_matches(f, patterns)]
+    if not relevant:
+        return False, "no changed file matches this member's paths"
+    others = [f for f in relevant if f != LOCKFILE]
+    if others:
+        shown = ", ".join(others[:5]) + (" ..." if len(others) > 5 else "")
+        return True, f"member-relevant files changed: {shown}"
+
+    old, new = lock_at(base, root), lock_at(head, root)
+    if old is None or new is None:
+        return True, "uv.lock changed and could not be read at both revisions"
+    before, after = closure(old, member_name), closure(new, member_name)
+    if member_name not in after:
+        return True, f"{member_name} is not in the head uv.lock"
+    if before == after:
+        return False, f"uv.lock changed, but not {member_name}'s resolved dependencies"
+    differing = sorted(n for n in before.keys() | after.keys() if before.get(n) != after.get(n))
+    shown = ", ".join(differing[:8]) + (" ..." if len(differing) > 8 else "")
+    return True, f"uv.lock changes {member_name}'s resolved dependencies: {shown}"
+
+
+def main(argv: list[str] | None = None) -> int:
+    parser = argparse.ArgumentParser(description=__doc__.split("\n", 1)[0])
+    parser.add_argument(
+        "--workflow",
+        required=True,
+        help="caller workflow, e.g. .github/workflows/lib-x.quality.yml",
+    )
+    parser.add_argument("--member-path", required=True, help="e.g. libs/algosystem")
+    parser.add_argument("--base", default="", help="commit to compare against; empty means run")
+    parser.add_argument("--head", default="HEAD")
+    args = parser.parse_args(argv)
+
+    def emit(run: bool, reason: str) -> int:
+        print(f"run={'true' if run else 'false'}")
+        print(f"reason={reason}")
+        return 0
+
+    if not args.base or set(args.base) == {"0"}:
+        return emit(True, "no base commit to compare against")
+
+    workflow = Path(args.workflow).as_posix()
+    callers = [wf for wf in load_quality_workflows() if wf.rel == workflow]
+    if not callers or not callers[0].pr_paths:
+        return emit(True, f"no pull_request.paths found in {workflow}")
+    members = [m for m in discover_members() if m.path == args.member_path]
+    if not members:
+        return emit(True, f"{args.member_path} is not a workspace member")
+
+    diff = _git("diff", "--name-only", args.base, args.head)
+    if diff is None:
+        return emit(True, f"could not diff {args.base}..{args.head}")
+    changed = [line for line in diff.splitlines() if line]
+    run, reason = decide(changed, callers[0].pr_paths, members[0].name, args.base, args.head)
+    return emit(run, reason)
+
+
+if __name__ == "__main__":
+    sys.exit(main())

tests/test_quality_needed.py

@@ -0,0 +1,150 @@
+"""Tests for platform/ci/quality_needed.py, the skip-unaffected-quality rule."""
+
+from __future__ import annotations
+
+import sys
+from pathlib import Path
+
+import pytest
+
+REPO_ROOT = Path(__file__).resolve().parents[1]
+sys.path.insert(0, str(REPO_ROOT / "platform" / "ci"))
+
+import quality_needed  # noqa: E402
+
+PATHS = [
+    "libs/lib-a/**",
+    "uv.lock",
+    "pyproject.toml",
+    ".github/workflows/_quality-python.yml",
+    ".github/workflows/lib-a.quality.yml",
+]
+
+
+def pkg(name: str, version: str, deps: tuple[str, ...] = (), **extra: object) -> dict:
+    entry: dict = {
+        "name": name,
+        "version": version,
+        "source": {"registry": "https://pypi.org/simple"},
+    }
+    if deps:
+        entry["dependencies"] = [{"name": d} for d in deps]
+    entry.update(extra)
+    return entry
+
+
+def lock(*packages: dict) -> dict:
+    root = {
+        "name": "workspace",
+        "version": "0",
+        "source": {"virtual": "."},
+        "dev-dependencies": {"dev": [{"name": "pytest"}]},
+    }
+    return {"package": [root, *packages]}
+
+
+BASE = lock(
+    {
+        "name": "lib-a",
+        "version": "0.1.0",
+        "source": {"editable": "libs/lib-a"},
+        "dependencies": [{"name": "sqlalchemy"}],
+    },
+    {
+        "name": "svc-b",
+        "version": "0.1.0",
+        "source": {"editable": "services/svc-b"},
+        "dependencies": [{"name": "aiohttp"}],
+    },
+    pkg("sqlalchemy", "1.4.54", ("greenlet",)),
+    pkg("greenlet", "3.5.5"),
+    pkg("aiohttp", "3.9.0"),
+    pkg("pytest", "8.0.0"),
+)
+
+
+def with_changes(**versions: str) -> dict:
+    out = {"package": []}
+    for entry in BASE["package"]:
+        entry = dict(entry)
+        if entry["name"] in versions:
+            entry["version"] = versions[entry["name"]]
+        out["package"].append(entry)
+    return out
+
+
+@pytest.fixture
+def locks(monkeypatch: pytest.MonkeyPatch):
+    store: dict[str, dict | None] = {}
+    monkeypatch.setattr(quality_needed, "lock_at", lambda rev, root=None: store.get(rev))
+    return store
+
+
+def decide(changed: list[str]) -> tuple[bool, str]:
+    return quality_needed.decide(changed, PATHS, "lib-a", "base", "head")
+
+
+def test_member_code_change_runs(locks) -> None:
+    run, reason = decide(["libs/lib-a/src/x.py"])
+    assert run and "libs/lib-a/src/x.py" in reason
+
+
+def test_unrelated_change_skips(locks) -> None:
+    run, _ = decide(["services/svc-b/app.py", "README.md"])
+    assert not run
+
+
+def test_root_config_change_runs(locks) -> None:
+    # Root pyproject.toml carries ruff/mypy/pytest config shared by every member.
+    assert decide(["pyproject.toml", "uv.lock"])[0]
+
+
+def test_lock_change_in_another_members_deps_skips(locks) -> None:
+    locks["base"], locks["head"] = BASE, with_changes(aiohttp="3.14.1")
+    run, reason = decide(["uv.lock", "services/svc-b/pyproject.toml"])
+    assert not run and "not lib-a's" in reason
+
+
+def test_lock_change_in_own_transitive_dep_runs(locks) -> None:
+    locks["base"], locks["head"] = BASE, with_changes(greenlet="3.6.0")
+    run, reason = decide(["uv.lock"])
+    assert run and "greenlet" in reason
+
+
+def test_lock_change_in_ci_tooling_runs(locks) -> None:
+    # The workspace root's dev group (pytest, ruff, mypy) runs every member's checks.
+    locks["base"], locks["head"] = BASE, with_changes(pytest="9.0.0")
+    assert decide(["uv.lock"])[0]
+
+
+def test_unreadable_lock_fails_open(locks) -> None:
+    locks["base"], locks["head"] = None, BASE
+    run, reason = decide(["uv.lock"])
+    assert run and "could not be read" in reason
+
+
+def test_no_base_commit_runs(capsys: pytest.CaptureFixture[str]) -> None:
+    quality_needed.main(
+        ["--workflow", ".github/workflows/x.quality.yml", "--member-path", "libs/x", "--base", ""]
+    )
+    assert "run=true" in capsys.readouterr().out
+
+
+def test_zero_sha_base_runs(capsys: pytest.CaptureFixture[str]) -> None:
+    quality_needed.main(["--workflow", "x", "--member-path", "x", "--base", "0" * 40])
+    assert "run=true" in capsys.readouterr().out
+
+
+def test_workflow_without_paths_runs(capsys: pytest.CaptureFixture[str]) -> None:
+    # Deploy/publish callers have no pull_request.paths: always run.
+    quality_needed.main(
+        [
+            "--workflow",
+            ".github/workflows/svc-data-ngin.deploy.yml",
+            "--member-path",
+            "services/data-ngin",
+            "--base",
+            "HEAD",
+        ]
+    )
+    assert "run=true" in capsys.readouterr().out