diff --git a/.github/scripts/pull-request-dashboard/publish_dashboard.py b/.github/scripts/pull-request-dashboard/publish_dashboard.py index e8faed16612f..6ffcefa941dd 100644 --- a/.github/scripts/pull-request-dashboard/publish_dashboard.py +++ b/.github/scripts/pull-request-dashboard/publish_dashboard.py @@ -25,7 +25,7 @@ from dashboard_contracts import StoredDashboardResult from pull_request_author import effective_author, is_copilot_author from pull_request_source import normalize_commits, normalize_pull_request -from render import render_pr_tables +from render import ISSUE_BODY_MAX_CHARS, render_pr_tables from state import ( dashboard_markdown_path, load_dashboard_state_cache, @@ -40,8 +40,6 @@ DASHBOARD_LABEL = "dashboard" DASHBOARD_LABEL_COLOR = "cfd3d7" DASHBOARD_LABEL_DESCRIPTION = "Pull request dashboard" -ISSUE_BODY_MAX_CHARS = 65536 -LARGE_REPO_MAX_ROWS_PER_SECTION = 100 def _parse_string_list_json(raw: str, source: str) -> list[str]: @@ -225,7 +223,6 @@ def add_draft_since(repo: str, prs: list[dict[str, Any]]) -> None: def render_dashboard_markdown( repo: str, - large_repo: bool, labels_to_display: list[str] | None = None, approver_teams: list[str] | None = None, ) -> Path: @@ -239,16 +236,26 @@ def render_dashboard_markdown( else () ) prs = list_open_prs(repo) - if not large_repo: - add_draft_authors(repo, prs) - add_draft_since(repo, prs) open_non_draft_pr_numbers = {p["number"] for p in prs if not p.get("isDraft")} results = results_from_dashboard_state(dashboard_state, open_non_draft_pr_numbers) + prs = publishable_prs(prs, results) + skip_drafts = False + if any(pr.get("isDraft") for pr in prs): + non_draft_md = render_pr_tables( + [pr for pr in prs if not pr.get("isDraft")], + results, + max_chars=None, + labels_to_display=labels_to_display, + approver_logins=approver_logins, + ) + skip_drafts = len(non_draft_md) >= ISSUE_BODY_MAX_CHARS + if not skip_drafts: + add_draft_authors(repo, prs) + add_draft_since(repo, prs) md = render_pr_tables( - publishable_prs(prs, results), + prs, results, - max_rows_per_section=LARGE_REPO_MAX_ROWS_PER_SECTION if large_repo else None, - skip_drafts=large_repo, + skip_drafts=skip_drafts, labels_to_display=labels_to_display, approver_logins=approver_logins, ) @@ -271,7 +278,6 @@ def render_dashboard_markdown( def publish_accepted_dashboard( repo: str, state_branch_name: str, - large_repo: bool, labels_to_display: list[str] | None = None, approver_teams: list[str] | None = None, ) -> None: @@ -283,7 +289,6 @@ def publish_accepted_dashboard( repo, render_dashboard_markdown( repo, - large_repo, labels_to_display, approver_teams, ), @@ -301,7 +306,7 @@ def main() -> int: parser.add_argument( "--large-repo", action="store_true", - help="apply large-repo rendering presets", + help="deprecated compatibility flag; dashboard sizing is always automatic", ) parser.add_argument( "--labels-to-display-json", @@ -314,7 +319,6 @@ def main() -> int: publish_accepted_dashboard( repo, args.state_branch, - args.large_repo, parse_labels_to_display_json(args.labels_to_display_json), parse_approver_teams_json(os.environ.get(APPROVER_TEAMS_ENV) or "[]"), ) diff --git a/.github/scripts/pull-request-dashboard/render.py b/.github/scripts/pull-request-dashboard/render.py index 5cf99b6a8344..71f5975ad4b8 100644 --- a/.github/scripts/pull-request-dashboard/render.py +++ b/.github/scripts/pull-request-dashboard/render.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections.abc import Iterable -from dataclasses import replace +from dataclasses import dataclass, replace from datetime import datetime from fnmatch import fnmatchcase from typing import Any @@ -26,10 +26,7 @@ ) -def _limit_rows(rows: list[Any], max_rows: int | None) -> tuple[list[Any], int]: - if max_rows is None or max_rows <= 0 or len(rows) <= max_rows: - return rows, 0 - return rows[:max_rows], len(rows) - max_rows +ISSUE_BODY_MAX_CHARS = 65536 def _truncation_note(count: int) -> str: @@ -37,6 +34,37 @@ def _truncation_note(count: int) -> str: return f"_More {count} {plural} not shown_" +@dataclass +class _PrSection: + heading: str + table_header: tuple[str, str] + rows: list[str] + omitted: int = 0 + + def lines(self) -> list[str]: + lines = [self.heading, ""] + if self.rows: + lines.extend(self.table_header) + lines.extend(self.rows) + lines.append("") + if self.omitted: + lines.extend([_truncation_note(self.omitted), ""]) + return lines + + def trim_last_row(self) -> int: + previous_footer_chars = ( + len(_truncation_note(self.omitted)) + 2 if self.omitted else 0 + ) + removed_chars = len(self.rows.pop()) + 1 + self.omitted += 1 + if not self.rows: + removed_chars += sum(len(line) + 1 for line in self.table_header) + 1 + return ( + removed_chars + previous_footer_chars + - len(_truncation_note(self.omitted)) - 2 + ) + + def _pr_cell_text( pr: dict[str, Any], labels_to_display: list[str] | None = None, @@ -105,20 +133,24 @@ def author_cell_text( return f"{author} {badge}" if badge else author -def render_draft_pr_section( +def _draft_pr_section( prs: list[dict[str, Any]], - max_rows_per_section: int | None = None, labels_to_display: list[str] | None = None, approver_logins: frozenset[str] = frozenset(), -) -> list[str]: + skip_drafts: bool = False, +) -> _PrSection | None: drafts = [p for p in prs if p.get("isDraft")] if not drafts: - return [] + return None + section = _PrSection( + "## Draft pull requests", + ("| PR | Author | Draft age |", "|---|---|:---:|"), + [], + ) + if skip_drafts: + section.omitted = len(drafts) + return section drafts.sort(key=lambda p: p.get("draftSince") or p.get("createdAt") or "") - drafts, truncated = _limit_rows(drafts, max_rows_per_section) - lines = ["## Draft pull requests", ""] - lines.append("| PR | Author | Draft age |") - lines.append("|---|---|:---:|") for pr in drafts: author = author_cell_text( pr, @@ -130,14 +162,19 @@ def render_draft_pr_section( ) # GitHub autolinks same-repo PR numbers; avoid full URLs so large # dashboards can show more PRs before hitting the issue body limit. - lines.append( + section.rows.append( f"| {_pr_cell_text(pr, labels_to_display)} | {author} | {draft_age} |" ) - lines.append("") - if truncated: - lines.append(_truncation_note(truncated)) - lines.append("") - return lines + return section + + +def render_draft_pr_section( + prs: list[dict[str, Any]], + labels_to_display: list[str] | None = None, + approver_logins: frozenset[str] = frozenset(), +) -> list[str]: + section = _draft_pr_section(prs, labels_to_display, approver_logins) + return section.lines() if section is not None else [] def ci_cell(facts: DashboardFacts) -> str: @@ -275,25 +312,18 @@ def reviewers_cell_text(facts: DashboardFacts) -> str: def render_pr_tables( prs: list[dict[str, Any]], results: Iterable[StoredDashboardResult], - max_rows_per_section: int | None = None, + max_chars: int | None = ISSUE_BODY_MAX_CHARS, skip_drafts: bool = False, labels_to_display: list[str] | None = None, approver_logins: frozenset[str] = frozenset(), ) -> str: + if max_chars is not None and max_chars <= 0: + raise ValueError("max_chars must be positive") results_by_number = { result.pr_number: result for result in results } source_url = "https://github.com/open-telemetry/shared-workflows/blob/main/.github/scripts/pull-request-dashboard/dashboard.py" - draft_note = ( - "Draft PRs are omitted to keep this dashboard concise." - if skip_drafts - else "Draft PRs are listed separately." - ) - grouping_note = ( - f"Open non-draft PRs grouped by who is expected to act next. {draft_note} The grouping is " - f"partly performed by an LLM ([source]({source_url})) and could contain mistakes." - ) author_note = ( "Author column: 🆕 first-time contributor · 🔭 open-telemetry member · " "🛠️ approver or maintainer · 🤖 bot." @@ -308,18 +338,6 @@ def render_pr_tables( "CI column: ✅ passing · ⏳ running · ❌ failing · " "🔐 workflow action required." ) - out: list[str] = [ - "> [!NOTE]", - f"> {grouping_note}", - ">", - f"> {author_note}", - ">", - f"> {reviewers_note}", - ">", - f"> {ci_note}", - "", - ] - by_route: dict[str, list[dict[str, Any]]] = {} for pr in prs: if pr.get("isDraft"): @@ -340,16 +358,20 @@ def row_sort_key(pr: dict[str, Any]) -> tuple[int, int]: activity = age_seconds(facts) return (activity if activity is not None else -1, pr["number"]) + sections: list[_PrSection] = [] for route in ROUTE_ORDER: rows = by_route.get(route) or [] if not rows: continue rows.sort(key=row_sort_key, reverse=True) - rows, truncated = _limit_rows(rows, max_rows_per_section) - out.append(f"## {route_label(route)}") - out.append("") - out.append("| PR | Author | Reviewers | CI | Conflicts | Age |") - out.append("|---|---|---|:---:|:---:|:---:|") + section = _PrSection( + f"## {route_label(route)}", + ( + "| PR | Author | Reviewers | CI | Conflicts | Age |", + "|---|---|---|:---:|:---:|:---:|", + ), + [], + ) for pr in rows: number = pr["number"] res = results_by_number.get(number) @@ -364,20 +386,77 @@ def row_sort_key(pr: dict[str, Any]) -> tuple[int, int]: # GitHub autolinks same-repo PR numbers; avoid full URLs so large # dashboards can show more PRs before hitting the issue body limit. pr_cell = _pr_cell_text(pr, labels_to_display) - out.append( + section.rows.append( f"| {pr_cell} | {author} | {reviewers_cell} | {ci_cell(facts)} | " f"{conflicts_cell(facts)} | {activity_cell} |" ) - out.append("") + sections.append(section) + + draft_section = _draft_pr_section( + prs, labels_to_display, approver_logins, skip_drafts, + ) + if draft_section is not None: + sections.append(draft_section) + truncated = any(section.omitted for section in sections) + + def intro_lines() -> list[str]: + draft_note = ( + "All drafts are omitted to fit GitHub's issue-body limit." + if draft_section is not None and not draft_section.rows + else "Draft PRs are listed separately." + ) + grouping_note = ( + f"Open non-draft PRs grouped by who is expected to act next. {draft_note} The grouping is " + f"partly performed by an LLM ([source]({source_url})) and could contain mistakes." + ) + lines = [ + "> [!NOTE]", + f"> {grouping_note}", + ">", + f"> {author_note}", + ">", + f"> {reviewers_note}", + ">", + f"> {ci_note}", + "", + ] if truncated: - out.append(_truncation_note(truncated)) - out.append("") - - if not skip_drafts: - out.extend(render_draft_pr_section( - prs, - max_rows_per_section, - labels_to_display, - approver_logins, - )) - return "\n".join(out) + "\n" + lines.extend([ + "> [!WARNING]", + "> This dashboard is truncated to fit GitHub's issue-body limit. " + "Sections are trimmed from bottom to top. " + "The longest-waiting PRs in each section are kept.", + "", + ]) + return lines + + def markdown() -> str: + lines = intro_lines() + for section in sections: + lines.extend(section.lines()) + return "\n".join(lines) + "\n" + + body = markdown() + if max_chars is None or len(body) <= max_chars: + return body + + intro_chars = sum(len(line) + 1 for line in intro_lines()) + truncated = True + new_intro_chars = sum(len(line) + 1 for line in intro_lines()) + body_chars = len(body) + new_intro_chars - intro_chars + intro_chars = new_intro_chars + for section in reversed(sections): + while section.rows and body_chars > max_chars: + body_chars -= section.trim_last_row() + new_intro_chars = sum(len(line) + 1 for line in intro_lines()) + body_chars += new_intro_chars - intro_chars + intro_chars = new_intro_chars + if body_chars <= max_chars: + break + + body = markdown() + if len(body) > max_chars: + raise RuntimeError( + f"dashboard headings and omission notices exceed the {max_chars}-character limit" + ) + return body diff --git a/.github/scripts/pull-request-dashboard/test_owned_execution.py b/.github/scripts/pull-request-dashboard/test_owned_execution.py index c1d0d8d27db5..45d787989ca4 100644 --- a/.github/scripts/pull-request-dashboard/test_owned_execution.py +++ b/.github/scripts/pull-request-dashboard/test_owned_execution.py @@ -258,7 +258,7 @@ def test_enqueue_requires_a_trackable_acceptance(self) -> None: def test_entry_inputs_preserve_target_and_head_validation(self) -> None: with tempfile.TemporaryDirectory() as directory: config = Path(directory) / "config.json" - config.write_text('[{"name":"a"},{"name":"b","large_repo":true}]', encoding="utf-8") + config.write_text('[{"name":"a"},{"name":"b"}]', encoding="utf-8") self.assertEqual( [(item["repository"], item["kind"], item["triggerEvent"]) for item in requests(config, "", "", "", "schedule")], [("a", "backfill", "schedule"), ("b", "backfill", "schedule")], @@ -448,7 +448,7 @@ def test_publication_failure_repeats_calculation_and_delivery_before_success(sel def test_bounded_initial_backfill_continues_until_full_publication(self) -> None: with tempfile.TemporaryDirectory() as directory: config = Path(directory) / "config.json" - config.write_text('[{"name":"example","large_repo":true}]', encoding="utf-8") + config.write_text('[{"name":"example"}]', encoding="utf-8") processor = DashboardBatchProcessor(config) item = WorkItem("example", None, ( Claim("example#backfill", 1, "example", None, "", 0, ("schedule",), "backfill"), @@ -467,9 +467,65 @@ def test_bounded_initial_backfill_continues_until_full_publication(self) -> None self.assertEqual(second[0]["outcome"], "success") self.assertEqual(update.call_args.args[1], None) self.assertEqual(update.call_args.args[4]["PREPARE_AUTHOR_NUDGES"], "true") - self.assertTrue(publish.call_args.args[2]["large_repo"]) + self.assertEqual("example", publish.call_args.args[2]["name"]) receipt.assert_called_once() + def test_publication_only_passes_repository_state_and_labels(self) -> None: + with tempfile.TemporaryDirectory() as directory: + config = Path(directory) / "config.json" + config.write_text('[{"name":"example"}]', encoding="utf-8") + processor = DashboardBatchProcessor(config) + env = {"APPROVER_TEAMS_JSON": '["example-approvers"]'} + with mock.patch.object(processor, "_run") as run: + processor._publish( + "open-telemetry/example", + "dashboard-state", + {"labels_to_display": ["size/*"]}, + env, + ) + run.assert_called_once_with( + [ + processor.python_executable, + str(processor.script_dir / "publish_dashboard.py"), + "--state-branch", + "dashboard-state", + "--repo", + "open-telemetry/example", + "--labels-to-display-json", + '["size/*"]', + ], + env=env, + ) + + def test_collector_contrib_preserves_sizing_flag_for_pinned_stable_publisher(self) -> None: + config_path = Path(__file__).with_name("repositories.json") + processor = DashboardBatchProcessor(config_path, script_dir=Path("promoted-code")) + config = processor.config["opentelemetry-collector-contrib"] + self.assertIs(True, config["large_repo"]) + + with mock.patch.object(processor, "_run") as run: + processor._publish( + "open-telemetry/opentelemetry-collector-contrib", + "dashboard-state", + config, + {}, + ) + + run.assert_called_once_with( + [ + processor.python_executable, + str(Path("promoted-code") / "publish_dashboard.py"), + "--state-branch", + "dashboard-state", + "--repo", + "open-telemetry/opentelemetry-collector-contrib", + "--labels-to-display-json", + "[]", + "--large-repo", + ], + env={}, + ) + def test_backfill_failure_can_publish_accepted_partial_state_but_never_succeeds(self) -> None: with tempfile.TemporaryDirectory() as directory: config = Path(directory) / "config.json" diff --git a/.github/scripts/pull-request-dashboard/test_publish_dashboard.py b/.github/scripts/pull-request-dashboard/test_publish_dashboard.py index e1deead9e375..55f14bad1af2 100644 --- a/.github/scripts/pull-request-dashboard/test_publish_dashboard.py +++ b/.github/scripts/pull-request-dashboard/test_publish_dashboard.py @@ -217,7 +217,6 @@ def test_publishes_from_read_only_accepted_state( publish_dashboard.publish_accepted_dashboard( "open-telemetry/example", "state-branch", - True, ["size/*"], ["example-approvers"], ) @@ -226,7 +225,6 @@ def test_publishes_from_read_only_accepted_state( set_state_dir.assert_called_once_with(checkout_dir / "example") render_dashboard_markdown.assert_called_once_with( "open-telemetry/example", - True, ["size/*"], ["example-approvers"], ) @@ -253,7 +251,6 @@ def test_worker_advancement_does_not_retry_a_pinned_snapshot( publish_dashboard.publish_accepted_dashboard( "open-telemetry/example", "state-branch", - False, ) self.assertEqual( @@ -292,7 +289,6 @@ def test_main_passes_approver_teams_from_the_environment( publish_accepted.assert_called_once_with( "open-telemetry/example", "state", - False, [], ["example-approvers"], ) @@ -314,7 +310,41 @@ def test_main_defaults_to_no_approver_teams(self, publish_accepted: object) -> N ): self.assertEqual(0, publish_dashboard.main()) - self.assertEqual([], publish_accepted.call_args.args[4]) + self.assertEqual([], publish_accepted.call_args.args[3]) + + @patch.object(publish_dashboard, "publish_accepted_dashboard") + def test_main_accepts_legacy_large_repo_flag_without_changing_publication( + self, + publish_accepted: object, + ) -> None: + for flags in ([], ["--large-repo"]): + with self.subTest(flags=flags): + publish_accepted.reset_mock() + with ( + patch.object( + sys, + "argv", + [ + "publish_dashboard.py", + "--repo", "example", + "--state-branch", "state", + "--labels-to-display-json", '["size/*"]', + *flags, + ], + ), + patch.dict( + os.environ, + {"APPROVER_TEAMS_JSON": '["example-approvers"]'}, + ), + ): + self.assertEqual(0, publish_dashboard.main()) + + publish_accepted.assert_called_once_with( + "open-telemetry/example", + "state", + ["size/*"], + ["example-approvers"], + ) def test_passes_labels_and_approvers_to_renderer(self) -> None: prs = [ @@ -359,7 +389,6 @@ def test_passes_labels_and_approvers_to_renderer(self) -> None: output_path, publish_dashboard.render_dashboard_markdown( "open-telemetry/example", - False, ["size/*"], ["example-approvers"], ), @@ -372,7 +401,6 @@ def test_passes_labels_and_approvers_to_renderer(self) -> None: render_pr_tables.assert_called_once_with( prs, results, - max_rows_per_section=None, skip_drafts=False, labels_to_display=["size/*"], approver_logins=frozenset({"approver"}), @@ -397,7 +425,6 @@ def test_skips_team_lookup_without_approver_teams(self) -> None: ): publish_dashboard.render_dashboard_markdown( "open-telemetry/example", - False, ) load_team_members.assert_not_called() @@ -445,7 +472,6 @@ def test_renders_effective_draft_authors_from_the_open_pull_listing(self) -> Non ): path = publish_dashboard.render_dashboard_markdown( "open-telemetry/example", - False, ) markdown = path.read_text(encoding="utf-8") @@ -456,7 +482,7 @@ def test_renders_effective_draft_authors_from_the_open_pull_listing(self) -> Non "/repos/open-telemetry/example/pulls/8/commits?per_page=1" ) - def test_large_repo_skips_draft_author_and_age_fetches(self) -> None: + def test_oversized_non_draft_dashboard_skips_draft_metadata_fetches(self) -> None: with tempfile.TemporaryDirectory() as tmp: output_path = Path(tmp) / "dashboard.md" with ( @@ -468,11 +494,23 @@ def test_large_repo_skips_draft_author_and_age_fetches(self) -> None: patch.object( publish_dashboard, "list_open_prs", - return_value=[{ - "number": 7, - "isDraft": True, - "author": {"login": "Copilot"}, - }], + return_value=[ + { + "number": 1, + "title": "x" * publish_dashboard.ISSUE_BODY_MAX_CHARS, + "isDraft": False, + }, + { + "number": 7, + "isDraft": True, + "author": {"login": "Copilot"}, + }, + ], + ), + patch.object( + publish_dashboard, + "results_from_dashboard_state", + return_value=(stored_dashboard_result(1),), ), patch.object(publish_dashboard, "add_draft_authors") as add_authors, patch.object(publish_dashboard, "add_draft_since") as add_since, @@ -484,14 +522,39 @@ def test_large_repo_skips_draft_author_and_age_fetches(self) -> None: ): path = publish_dashboard.render_dashboard_markdown( "open-telemetry/example", - True, ) markdown = path.read_text(encoding="utf-8") - self.assertNotIn("## Draft pull requests", markdown) + self.assertIn("## Draft pull requests\n\n_More 1 PR not shown_", markdown) + self.assertIn("All drafts are omitted to fit GitHub's issue-body limit.", markdown) + self.assertNotIn("| #7 ", markdown) + self.assertLessEqual(len(markdown), publish_dashboard.ISSUE_BODY_MAX_CHARS) add_authors.assert_not_called() add_since.assert_not_called() + def test_publisher_retains_the_final_size_guard(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + with ( + patch.object( + publish_dashboard, + "load_dashboard_state_cache", + return_value=dashboard_state(), + ), + patch.object(publish_dashboard, "list_open_prs", return_value=[]), + patch.object( + publish_dashboard, + "render_pr_tables", + return_value="x" * (publish_dashboard.ISSUE_BODY_MAX_CHARS + 1), + ), + patch.object( + publish_dashboard, + "dashboard_markdown_path", + return_value=Path(tmp) / "dashboard.md", + ), + self.assertRaisesRegex(RuntimeError, "exceeds GitHub's"), + ): + publish_dashboard.render_dashboard_markdown("open-telemetry/example") + if __name__ == "__main__": unittest.main() diff --git a/.github/scripts/pull-request-dashboard/test_render.py b/.github/scripts/pull-request-dashboard/test_render.py index fd5991ea81ec..c9fe6d2fb205 100644 --- a/.github/scripts/pull-request-dashboard/test_render.py +++ b/.github/scripts/pull-request-dashboard/test_render.py @@ -1,5 +1,8 @@ from __future__ import annotations +from copy import deepcopy +from datetime import datetime, timezone +import re import unittest from unittest.mock import patch @@ -8,12 +11,218 @@ stored_dashboard_result, ) from render import ( + ISSUE_BODY_MAX_CHARS, author_cell_text, ci_cell, render_draft_pr_section, render_pr_tables, reviewers_cell_text, ) +from route_presentation import ROUTE_ORDER, route_label + + +class DashboardSizingTest(unittest.TestCase): + def setUp(self) -> None: + self.enterContext(patch("render.activity_age", return_value="1d")) + + def test_more_than_100_rows_and_drafts_are_shown_when_the_body_fits(self) -> None: + prs = [ + {"number": number, "title": "Feature", "isDraft": False} + for number in range(1, 151) + ] + [{"number": 151, "title": "Draft", "isDraft": True}] + results = tuple(stored_dashboard_result(number) for number in range(1, 151)) + + full = render_pr_tables(prs, results, max_chars=None) + markdown = render_pr_tables(prs, results) + + self.assertEqual(full, markdown) + self.assertLess(len(markdown), ISSUE_BODY_MAX_CHARS) + self.assertEqual(151, len(re.findall(r"^\| #\d+ ", markdown, re.MULTILINE))) + self.assertNotIn("[!WARNING]", markdown) + self.assertNotIn("not shown", markdown) + + def test_actual_character_limit_with_unicode_and_escaped_markdown(self) -> None: + for offset in (-1, 0, 1): + with self.subTest(offset=offset): + prs = [{ + "number": 1, + "title": "日本語 🧪 & | [x]", + "isDraft": False, + }] + results = (stored_dashboard_result(1),) + full = render_pr_tables(prs, results, max_chars=None) + prs[0]["title"] += "x" * (ISSUE_BODY_MAX_CHARS + offset - len(full)) + full = render_pr_tables(prs, results, max_chars=None) + markdown = render_pr_tables(prs, results) + + self.assertEqual(ISSUE_BODY_MAX_CHARS + offset, len(full)) + self.assertGreater(len(full.encode("utf-8")), ISSUE_BODY_MAX_CHARS) + self.assertLessEqual(len(markdown), ISSUE_BODY_MAX_CHARS) + if offset <= 0: + self.assertEqual(full, markdown) + self.assertNotIn("[!WARNING]", markdown) + else: + self.assertNotIn("| #1 ", markdown) + self.assertIn("## Waiting on authors\n\n_More 1 PR not shown_", markdown) + self.assertIn("[!WARNING]", markdown) + + def test_trims_sections_and_rows_in_reverse_display_order(self) -> None: + prs = [] + results = [] + routes = [route for route in ROUTE_ORDER if route != "transient-failure"] + for index, route in enumerate(routes): + for number in (index * 2 + 1, index * 2 + 2): + prs.append({ + "number": number, + "title": "x" * 1000, + "isDraft": False, + }) + if route != "unknown": + results.append(stored_dashboard_result( + number, + route, + facts=dashboard_facts( + waiting_since=f"2026-07-{number:02d}T00:00:00Z", + ), + )) + prs.extend([ + { + "number": number, + "title": "x" * 1000, + "isDraft": True, + "draftSince": f"2026-07-{number:02d}T00:00:00Z", + } + for number in (9, 10) + ]) + prs.reverse() + original_prs = deepcopy(prs) + full = render_pr_tables(prs, results, max_chars=None) + visible_counts = set() + + for budget in range(1500, len(full) + 500, 250): + with self.subTest(budget=budget): + markdown = render_pr_tables(prs, results, max_chars=budget) + visible = [ + int(number) + for number in re.findall(r"^\| #(\d+) ", markdown, re.MULTILINE) + ] + visible_counts.add(len(visible)) + self.assertEqual([1, 2, 3, 4, 5, 6, 8, 7, 9, 10][:len(visible)], visible) + self.assertLessEqual(len(markdown), budget) + for route in routes: + self.assertIn(f"## {route_label(route)}", markdown) + self.assertIn("## Draft pull requests", markdown) + omitted = sum( + int(count) + for count in re.findall(r"_More (\d+) PRs? not shown_", markdown) + ) + self.assertEqual(10 - len(visible), omitted) + self.assertEqual(len(visible) < 10, "[!WARNING]" in markdown) + self.assertEqual(set(range(11)), visible_counts) + self.assertEqual(original_prs, prs) + + def test_partial_draft_section_keeps_the_oldest_time_in_draft(self) -> None: + prs = [ + { + "number": 1, + "title": "x" * 1000, + "isDraft": True, + "draftSince": "2026-07-10T00:00:00Z", + "createdAt": "2026-07-01T00:00:00Z", + }, + { + "number": 2, + "title": "x" * 1000, + "isDraft": True, + "draftSince": "2026-07-01T00:00:00Z", + "createdAt": "2026-07-02T00:00:00Z", + }, + ] + full = render_pr_tables(prs, (), max_chars=None) + + markdown = render_pr_tables(prs, (), max_chars=len(full) - 1) + + self.assertNotIn("| #1 ", markdown) + self.assertIn("| #2 ", markdown) + self.assertIn("_More 1 PR not shown_", markdown) + self.assertNotIn("All drafts are omitted", markdown) + self.assertLessEqual(len(markdown), len(full) - 1) + + def test_fully_omitted_drafts_keep_their_count_without_an_empty_table(self) -> None: + prs = [ + {"number": number, "title": "x" * 1000, "isDraft": True} + for number in range(1, 13) + ] + + markdown = render_pr_tables(prs, (), max_chars=1500) + + self.assertIn("All drafts are omitted to fit GitHub's issue-body limit.", markdown) + self.assertIn("## Draft pull requests\n\n_More 12 PRs not shown_", markdown) + self.assertNotIn("| PR | Author | Draft age |", markdown) + self.assertNotIn("| #", markdown) + self.assertLessEqual(len(markdown), 1500) + + def test_waiting_age_fallback_ties_and_unknown_ages_preserve_sorting(self) -> None: + prs = [ + {"number": number, "title": "x" * 1000, "isDraft": False} + for number in range(1, 6) + ] + results = ( + stored_dashboard_result(1, facts=dashboard_facts( + waiting_since="2026-07-01T00:00:00Z", + last_activity_at="2026-07-03T00:00:00Z", + )), + stored_dashboard_result(2, facts=dashboard_facts( + waiting_since="2026-07-01T00:00:00Z", + )), + stored_dashboard_result(3, facts=dashboard_facts( + waiting_since="2026-07-02T00:00:00Z", + last_activity_at="2026-06-01T00:00:00Z", + )), + stored_dashboard_result(4), + stored_dashboard_result(5, facts=dashboard_facts( + last_activity_at="2026-06-30T00:00:00Z", + )), + ) + now = datetime(2026, 11, 3, tzinfo=timezone.utc) + with patch( + "render.seconds_since", + side_effect=lambda ts: int((now - ts).total_seconds()) if ts else None, + ): + full = render_pr_tables(prs, results, max_chars=None) + markdown = render_pr_tables(prs, results, max_chars=len(full) - 1100) + + self.assertEqual( + ["5", "2", "1"], + re.findall(r"^\| #(\d+) ", markdown, re.MULTILINE), + ) + self.assertIn("_More 2 PRs not shown_", markdown) + self.assertLessEqual(len(markdown), len(full) - 1100) + + def test_warning_and_footer_overhead_is_included_in_the_budget(self) -> None: + prs = [ + {"number": number, "title": "Small", "isDraft": False} + for number in range(1, 21) + ] + results = tuple(stored_dashboard_result(number) for number in range(1, 21)) + full = render_pr_tables(prs, results, max_chars=None) + + markdown = render_pr_tables(prs, results, max_chars=len(full) - 1) + + self.assertLessEqual(len(markdown), len(full) - 1) + self.assertIn("[!WARNING]", markdown) + self.assertGreater( + int(re.search(r"_More (\d+) PRs not shown_", markdown).group(1)), + 1, + ) + + def test_impossible_budget_fails_explicitly(self) -> None: + with self.assertRaisesRegex(RuntimeError, "headings and omission notices exceed"): + render_pr_tables([], (), max_chars=1) + for budget in (0, -1): + with self.subTest(budget=budget): + with self.assertRaisesRegex(ValueError, "max_chars must be positive"): + render_pr_tables([], (), max_chars=budget) class RenderTest(unittest.TestCase): diff --git a/pull-request-dashboard/README.md b/pull-request-dashboard/README.md index f7c20c691473..8297012cccec 100644 --- a/pull-request-dashboard/README.md +++ b/pull-request-dashboard/README.md @@ -159,7 +159,7 @@ There is no direct-dispatch fallback or return-to-legacy execution mode. ## Dashboard columns -The dashboard groups open non-draft pull requests by who is expected to act next (e.g. *Waiting on reviewers*, *Waiting on authors*, *Waiting on maintainers*). Draft PRs are listed separately at the bottom unless `large_repo` rendering is enabled. Within each group, rows are sorted longest-waiting first. Every row has these six columns: +The dashboard groups open non-draft pull requests by who is expected to act next (e.g. *Waiting on reviewers*, *Waiting on authors*, *Waiting on maintainers*). Draft PRs are listed separately at the bottom. Within each group, rows are sorted longest-waiting first. Every row has these six columns: - **PR** — Pull request number and title, followed by any configured matching labels. The number autolinks to the PR on GitHub. Configured labels are rendered inline for both active and draft PRs. - **Author** — Effective author of the PR, including in the draft section. For Copilot-authored PRs, this is the human assignee or first-commit committer when available. The login is followed by at most one badge, checked in this order: @@ -242,7 +242,13 @@ Fields: | `close_stale_exempt_labels` | no | Case-sensitive label names that exempt a PR from `close_stale_after_days`, for example `["never-stale"]`. Defaults to `[]`. | | `slack_channel` | no | Slack channel for notifications. Omit to skip Slack processing for this repository. | | `slack_user_mapping` | no | Map of GitHub login to Slack user ID for at-mentions. | -| `large_repo` | no | If `true`, apply rendering presets that keep the dashboard body under GitHub's 65,536-character issue-body limit: cap each section (each *Waiting on …* table and the *Draft pull requests* table) at 100 rows, and omit the *Draft pull requests* section entirely. Truncated sections get a `_More X PRs not shown_` footer. Defaults to `false` (no cap, drafts shown). Enable this for very large repos with hundreds of PRs. | +| `large_repo` | no | Deprecated. Pinned releases without automatic sizing use `true` to cap each non-draft section at 100 rows and omit drafts. The automatic publisher ignores this setting. Keep existing entries until automatic sizing has been promoted to stable. | + +Dashboard sizing is automatic. All rows, including drafts, are shown whenever the complete Markdown body fits GitHub's 65,536-character issue-body limit. Otherwise, complete rows are removed from the bottom of the dashboard: drafts first, then unknown PRs, transient GitHub failures, PRs waiting on authors, PRs waiting on reviewers, and finally PRs waiting on maintainers. Each lower section is exhausted before the next section is trimmed. Non-draft sections keep their longest-waiting PRs; the draft section keeps the PRs that have been drafts longest. + +A truncated dashboard includes a size-limit warning and a `_More X PRs not shown_` footer in each affected section. Section headings and omission counts remain even when all their rows are omitted. The introductory note explicitly says when all drafts are omitted. Notices and footers count toward the size limit. Truncation only affects the dashboard display, not PR processing, routing, or notifications. + +Automatic sizing follows the canary-to-stable release rollout. Until its release is promoted, stable repositories retain the pinned publisher's behavior. The queue forwards `--large-repo` for configured repositories to preserve their sizing protection; the automatic publisher accepts the flag as a no-op. Remove the legacy configuration entries, flag forwarding, and compatibility flag only after the automatic publisher is promoted to stable. `labels_to_display` only controls which labels are shown. It does not filter pull requests or affect dashboard routing, notifications, or status comments. All matching labels are displayed in the order returned by GitHub; a label matching more than one configured pattern is shown once.