From 3250f5c5b26a4ccadc5ef7c3372a28340fa6ae33 Mon Sep 17 00:00:00 2001 From: hannahwestra25 Date: Tue, 25 Aug 2026 16:05:14 -0400 Subject: [PATCH 1/2] FIX Restore docs build validation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/docs.yml | 10 ++-- build_scripts/gen_api_md.py | 58 ++++++++++++++----- build_scripts/resolve_docs_matrix.py | 31 ++++++++-- tests/unit/build_scripts/test_gen_api_md.py | 29 ++++++++++ .../build_scripts/test_resolve_docs_matrix.py | 31 ++++++++++ 5 files changed, 137 insertions(+), 22 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 63f6cce0a6..2539b441f4 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -75,6 +75,8 @@ jobs: run: | python -m build_scripts.resolve_docs_matrix \ --config .github/docs-versions.yml \ + --event-name "${{ github.event_name }}" \ + --github-sha "${{ github.sha }}" \ --github-output "$GITHUB_OUTPUT" # ------------------------------------------------------------------ @@ -96,11 +98,9 @@ jobs: ref: ${{ matrix.ref }} - name: Resolve commit SHA - # matrix.ref is a branch name (e.g. "releases/v0.13.0") that can move, - # so resolve to the actual commit SHA we just checked out. This is the - # primary cache key component: frozen release branches that don't move - # produce identical SHAs and hit the cache forever; main produces a - # new SHA on every push so it always rebuilds. + # matrix.ref is usually a branch name (e.g. "releases/v0.13.0") that can + # move; latest pull request builds use the event's merge SHA instead. + # Resolve either form to the actual checked-out SHA for the cache key. id: sha run: echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" diff --git a/build_scripts/gen_api_md.py b/build_scripts/gen_api_md.py index 1b9e81e84c..f14fde81a4 100644 --- a/build_scripts/gen_api_md.py +++ b/build_scripts/gen_api_md.py @@ -654,24 +654,56 @@ def _resolve_aliases(modules: list[dict], definition_index: dict, name_to_module def _expand_module(module: dict) -> list[dict]: - """Recursively expand pure-aggregate modules into their children. + """Expand an aggregate module to the public API page frontier. - A pure-aggregate module has only submodule members and no direct public API - (classes, functions, aliases). Its children are returned instead, recursing - further if a child is also a pure aggregate. + A module with unique direct API keeps its own page. If that module also + contains submodules, each submodule branch is expanded until it reaches the + first module with direct API. Direct members already represented on that + frontier are omitted, and submodule objects are removed from the retained + parent so each module and symbol is represented only once. """ members = module.get("members", []) - has_api = any(m.get("kind") in ("class", "function", "alias") for m in members) - submodules = [m for m in members if m.get("kind") == "module"] + direct_members = [member for member in members if member.get("kind") != "module"] + submodules = [member for member in members if member.get("kind") == "module"] + + if not submodules: + has_api = any(member.get("kind") in ("class", "function", "alias") for member in direct_members) + return [module] if has_api else [] + + frontier: list[dict] = [] + for submodule in submodules: + frontier.extend(_expand_module_frontier(submodule)) + + descendant_api = { + (member.get("kind"), member.get("name")) + for descendant in frontier + for member in descendant.get("members", []) + if member.get("kind") in ("class", "function", "alias") + } + unique_direct_members = [ + member + for member in direct_members + if member.get("kind") not in ("class", "function", "alias") + or (member.get("kind"), member.get("name")) not in descendant_api + ] + has_unique_api = any(member.get("kind") in ("class", "function", "alias") for member in unique_direct_members) + if has_unique_api: + return [{**module, "members": unique_direct_members}, *frontier] + return frontier + + +def _expand_module_frontier(module: dict) -> list[dict]: + """Expand pure aggregates, stopping at the first module with direct API.""" + members = module.get("members", []) + has_api = any(member.get("kind") in ("class", "function", "alias") for member in members) + submodules = [member for member in members if member.get("kind") == "module"] - if has_api or not submodules: - # Module has its own API, or is a leaf – keep it (filter empty later) + if has_api: return [module] - # Pure aggregate – recurse into children result: list[dict] = [] - for sub in submodules: - result.extend(_expand_module(sub)) + for submodule in submodules: + result.extend(_expand_module_frontier(submodule)) return result @@ -683,8 +715,8 @@ def collect_top_level_modules(api_json_dir: Path) -> list[dict]: for the public packages users import from, not for deeply nested internal submodules whose content is re-exported by the parent. - Pure-aggregate modules (those with only submodule members) are recursively - expanded so their children with real API surface get their own pages. + Aggregate modules are expanded to the first public API package on each + submodule branch. A mixed aggregate also keeps a page for its direct API. """ modules: list[dict] = [] for jf in sorted(api_json_dir.glob("*.json")): diff --git a/build_scripts/resolve_docs_matrix.py b/build_scripts/resolve_docs_matrix.py index f71e110d5b..574b3541a2 100644 --- a/build_scripts/resolve_docs_matrix.py +++ b/build_scripts/resolve_docs_matrix.py @@ -54,10 +54,32 @@ def load_config(config_path: Path) -> dict[str, Any]: return cfg -def build_outputs(cfg: dict[str, Any]) -> dict[str, str]: +def _resolve_build_ref(*, version: dict[str, Any], event_name: str | None, github_sha: str | None) -> str: + """Resolve the source revision for one docs matrix entry.""" + if event_name != "pull_request" or version["slug"] != "latest": + return version["ref"] + if not github_sha: + raise ValueError("github_sha is required when resolving the latest pull request docs build") + return github_sha + + +def build_outputs( + cfg: dict[str, Any], + *, + event_name: str | None = None, + github_sha: str | None = None, +) -> dict[str, str]: """Return the four GitHub Actions step outputs (all values are strings).""" versions = cfg["versions"] - matrix = {"include": [{"slug": v["slug"], "ref": v["ref"]} for v in versions]} + matrix = { + "include": [ + { + "slug": version["slug"], + "ref": _resolve_build_ref(version=version, event_name=event_name, github_sha=github_sha), + } + for version in versions + ] + } versions_json = { "default": cfg["default"], "stable": cfg["stable"], @@ -95,16 +117,17 @@ def main(argv: list[str] | None = None) -> int: type=Path, help="Path to the $GITHUB_OUTPUT file. If omitted, outputs are written to stdout.", ) + parser.add_argument("--event-name", help="GitHub event name used to resolve event-specific build refs.") + parser.add_argument("--github-sha", help="GitHub event commit SHA used for the latest pull request build.") args = parser.parse_args(argv) try: cfg = load_config(args.config.resolve()) + outputs = build_outputs(cfg, event_name=args.event_name, github_sha=args.github_sha) except (FileNotFoundError, ValueError) as e: print(f"error: {e}", file=sys.stderr) return 1 - outputs = build_outputs(cfg) - if args.github_output is not None: with args.github_output.open("a", encoding="utf-8") as f: write_outputs(f, outputs) diff --git a/tests/unit/build_scripts/test_gen_api_md.py b/tests/unit/build_scripts/test_gen_api_md.py index 15a1cec692..31f22ec705 100644 --- a/tests/unit/build_scripts/test_gen_api_md.py +++ b/tests/unit/build_scripts/test_gen_api_md.py @@ -12,6 +12,7 @@ _build_symbol_index, _class_anchor, _example_link_path, + _expand_module, _format_bases, _format_reexport_alias, _format_reexport_target, @@ -45,6 +46,34 @@ def _fake_module(name: str, members: list[dict]) -> dict: return {"name": name, "kind": "module", "members": members} +def test_expand_module_preserves_mixed_direct_api_and_public_frontier() -> None: + module = _fake_module( + "pyrit", + [ + _fake_function("show_versions"), + _fake_module( + "pyrit.common", + [ + _fake_function("resolve_lazy_export"), + _fake_class("CommonConfig"), + _fake_module("pyrit.common.internal", [_fake_class("InternalHelper")]), + ], + ), + _fake_function("resolve_lazy_export"), + _fake_module( + "pyrit.executor", + [_fake_module("pyrit.executor.attack", [_fake_class("AttackStrategy")])], + ), + ], + ) + + expanded = _expand_module(module) + + assert [item["name"] for item in expanded] == ["pyrit", "pyrit.common", "pyrit.executor.attack"] + assert expanded[0]["members"] == [_fake_function("show_versions")] + assert len({item["name"] for item in expanded}) == len(expanded) + + def test_anchor_helpers_produce_unique_labels() -> None: assert _class_anchor("pyrit.prompt_target", "PromptTarget") == "api-pyrit_prompt_target-PromptTarget" assert _function_anchor("pyrit.common", "validate_log_level") == "api-pyrit_common-validate_log_level" diff --git a/tests/unit/build_scripts/test_resolve_docs_matrix.py b/tests/unit/build_scripts/test_resolve_docs_matrix.py index 3d7f066581..57aafc9d30 100644 --- a/tests/unit/build_scripts/test_resolve_docs_matrix.py +++ b/tests/unit/build_scripts/test_resolve_docs_matrix.py @@ -68,6 +68,37 @@ def test_build_outputs_shape(module, tmp_path): assert len(payload["versions"]) == 3 +def test_build_outputs_uses_merge_sha_only_for_latest_pull_request(module, tmp_path): + cfg = module.load_config(_write_yaml(tmp_path, _VALID_YAML)) + + outputs = module.build_outputs(cfg, event_name="pull_request", github_sha="abc123") + + assert json.loads(outputs["matrix"]) == { + "include": [ + {"slug": "latest", "ref": "abc123"}, + {"slug": "0.13.0", "ref": "releases/v0.13.0"}, + {"slug": "0.12.1", "ref": "releases/v0.12.1"}, + ] + } + + +@pytest.mark.parametrize("event_name", ["push", "workflow_dispatch"]) +def test_build_outputs_uses_configured_refs_outside_pull_requests(module, tmp_path, event_name): + cfg = module.load_config(_write_yaml(tmp_path, _VALID_YAML)) + + outputs = module.build_outputs(cfg, event_name=event_name, github_sha="abc123") + + matrix = json.loads(outputs["matrix"]) + assert matrix["include"][0] == {"slug": "latest", "ref": "main"} + + +def test_build_outputs_requires_sha_for_latest_pull_request(module, tmp_path): + cfg = module.load_config(_write_yaml(tmp_path, _VALID_YAML)) + + with pytest.raises(ValueError, match="github_sha is required"): + module.build_outputs(cfg, event_name="pull_request") + + def test_outputs_are_single_line(module, tmp_path): """GH Actions step outputs are key=value lines; no embedded newlines.""" cfg = module.load_config(_write_yaml(tmp_path, _VALID_YAML)) From 5c2ad903e592fdcd0c69209dd9316395ff6a45d2 Mon Sep 17 00:00:00 2001 From: hannahwestra25 Date: Tue, 25 Aug 2026 16:19:51 -0400 Subject: [PATCH 2/2] FIX Select PR docs ref by base branch Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8b8833c-24dc-44bf-980b-8273b0f7ef4f --- .github/workflows/docs.yml | 1 + build_scripts/resolve_docs_matrix.py | 36 ++++++++++-- .../build_scripts/test_resolve_docs_matrix.py | 58 +++++++++++++++++-- 3 files changed, 84 insertions(+), 11 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 2539b441f4..725a009bd1 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -77,6 +77,7 @@ jobs: --config .github/docs-versions.yml \ --event-name "${{ github.event_name }}" \ --github-sha "${{ github.sha }}" \ + --github-base-ref "${{ github.base_ref }}" \ --github-output "$GITHUB_OUTPUT" # ------------------------------------------------------------------ diff --git a/build_scripts/resolve_docs_matrix.py b/build_scripts/resolve_docs_matrix.py index 574b3541a2..255e77cb80 100644 --- a/build_scripts/resolve_docs_matrix.py +++ b/build_scripts/resolve_docs_matrix.py @@ -54,12 +54,18 @@ def load_config(config_path: Path) -> dict[str, Any]: return cfg -def _resolve_build_ref(*, version: dict[str, Any], event_name: str | None, github_sha: str | None) -> str: +def _resolve_build_ref( + *, + version: dict[str, Any], + event_name: str | None, + github_sha: str | None, + github_base_ref: str | None, +) -> str: """Resolve the source revision for one docs matrix entry.""" - if event_name != "pull_request" or version["slug"] != "latest": + if event_name != "pull_request" or version["ref"] != github_base_ref: return version["ref"] if not github_sha: - raise ValueError("github_sha is required when resolving the latest pull request docs build") + raise ValueError("github_sha is required when resolving a pull request docs build") return github_sha @@ -68,14 +74,26 @@ def build_outputs( *, event_name: str | None = None, github_sha: str | None = None, + github_base_ref: str | None = None, ) -> dict[str, str]: """Return the four GitHub Actions step outputs (all values are strings).""" versions = cfg["versions"] + if event_name == "pull_request": + if not github_base_ref: + raise ValueError("github_base_ref is required when resolving a pull request docs build") + if not any(version["ref"] == github_base_ref for version in versions): + raise ValueError(f"pull request base ref {github_base_ref!r} is not configured in docs versions") + matrix = { "include": [ { "slug": version["slug"], - "ref": _resolve_build_ref(version=version, event_name=event_name, github_sha=github_sha), + "ref": _resolve_build_ref( + version=version, + event_name=event_name, + github_sha=github_sha, + github_base_ref=github_base_ref, + ), } for version in versions ] @@ -118,12 +136,18 @@ def main(argv: list[str] | None = None) -> int: help="Path to the $GITHUB_OUTPUT file. If omitted, outputs are written to stdout.", ) parser.add_argument("--event-name", help="GitHub event name used to resolve event-specific build refs.") - parser.add_argument("--github-sha", help="GitHub event commit SHA used for the latest pull request build.") + parser.add_argument("--github-sha", help="GitHub event commit SHA used for a pull request build.") + parser.add_argument("--github-base-ref", help="Pull request base branch matched to a configured docs version.") args = parser.parse_args(argv) try: cfg = load_config(args.config.resolve()) - outputs = build_outputs(cfg, event_name=args.event_name, github_sha=args.github_sha) + outputs = build_outputs( + cfg, + event_name=args.event_name, + github_sha=args.github_sha, + github_base_ref=args.github_base_ref, + ) except (FileNotFoundError, ValueError) as e: print(f"error: {e}", file=sys.stderr) return 1 diff --git a/tests/unit/build_scripts/test_resolve_docs_matrix.py b/tests/unit/build_scripts/test_resolve_docs_matrix.py index 57aafc9d30..edbbf9a53e 100644 --- a/tests/unit/build_scripts/test_resolve_docs_matrix.py +++ b/tests/unit/build_scripts/test_resolve_docs_matrix.py @@ -68,10 +68,15 @@ def test_build_outputs_shape(module, tmp_path): assert len(payload["versions"]) == 3 -def test_build_outputs_uses_merge_sha_only_for_latest_pull_request(module, tmp_path): +def test_build_outputs_uses_merge_sha_for_main_target_pull_request(module, tmp_path): cfg = module.load_config(_write_yaml(tmp_path, _VALID_YAML)) - outputs = module.build_outputs(cfg, event_name="pull_request", github_sha="abc123") + outputs = module.build_outputs( + cfg, + event_name="pull_request", + github_sha="abc123", + github_base_ref="main", + ) assert json.loads(outputs["matrix"]) == { "include": [ @@ -82,21 +87,64 @@ def test_build_outputs_uses_merge_sha_only_for_latest_pull_request(module, tmp_p } +def test_build_outputs_uses_merge_sha_for_release_target_pull_request(module, tmp_path): + cfg = module.load_config(_write_yaml(tmp_path, _VALID_YAML)) + + outputs = module.build_outputs( + cfg, + event_name="pull_request", + github_sha="abc123", + github_base_ref="releases/v0.13.0", + ) + + assert json.loads(outputs["matrix"]) == { + "include": [ + {"slug": "latest", "ref": "main"}, + {"slug": "0.13.0", "ref": "abc123"}, + {"slug": "0.12.1", "ref": "releases/v0.12.1"}, + ] + } + + @pytest.mark.parametrize("event_name", ["push", "workflow_dispatch"]) def test_build_outputs_uses_configured_refs_outside_pull_requests(module, tmp_path, event_name): cfg = module.load_config(_write_yaml(tmp_path, _VALID_YAML)) - outputs = module.build_outputs(cfg, event_name=event_name, github_sha="abc123") + outputs = module.build_outputs( + cfg, + event_name=event_name, + github_sha="abc123", + github_base_ref="releases/v0.13.0", + ) matrix = json.loads(outputs["matrix"]) assert matrix["include"][0] == {"slug": "latest", "ref": "main"} -def test_build_outputs_requires_sha_for_latest_pull_request(module, tmp_path): +def test_build_outputs_requires_sha_for_pull_request(module, tmp_path): cfg = module.load_config(_write_yaml(tmp_path, _VALID_YAML)) with pytest.raises(ValueError, match="github_sha is required"): - module.build_outputs(cfg, event_name="pull_request") + module.build_outputs(cfg, event_name="pull_request", github_base_ref="main") + + +def test_build_outputs_requires_base_ref_for_pull_request(module, tmp_path): + cfg = module.load_config(_write_yaml(tmp_path, _VALID_YAML)) + + with pytest.raises(ValueError, match="github_base_ref is required"): + module.build_outputs(cfg, event_name="pull_request", github_sha="abc123") + + +def test_build_outputs_rejects_unconfigured_pull_request_base_ref(module, tmp_path): + cfg = module.load_config(_write_yaml(tmp_path, _VALID_YAML)) + + with pytest.raises(ValueError, match="'releases/v9.9.9' is not configured"): + module.build_outputs( + cfg, + event_name="pull_request", + github_sha="abc123", + github_base_ref="releases/v9.9.9", + ) def test_outputs_are_single_line(module, tmp_path):