Add descriptions to validations and improve PR comment formatting (DataDog#23308)

* Add descriptions to validations and improve PR comment formatting

- Add a description field to ValidationConfig shown in summary tables
- Collapse the validation table in PR comments: all-pass wraps the
  entire table in a <details> block; partial failures show only failures
  up front with passed validations collapsed
- Keep the GitHub Actions step summary as a full flat table
- Remove redundant "View full run" link from PR comments

* Keep run URL in PR comment, only omit from step summary

* Add changelog entry

* Address review feedback: extract _build_preamble helper, add missing test coverage, minor cleanups

* Rename _post_pr_comment to _publish_report, single-pass partition of results, use extend over repeated appends

* Unify section builders into a single _build_report_section with collapsed flag

* Fix changelog entry number

* Warn about incomplete validations in PR comment and step summary

* Use descriptive variable names in results loop

* Make expected_count a required positional parameter

* Show names of incomplete validations instead of just the count

* Lint formatting

* Fix Windows test failure: read summary file with utf-8 encoding

Author: AAraKKe

ddev/changelog.d/23308.fixed

@@ -0,0 +1 @@
+Add descriptions to validations and improve PR comment formatting with collapsible sections

ddev/src/ddev/cli/validate/all/github.py

@@ -12,7 +12,7 @@
 
 if TYPE_CHECKING:
     from ddev.cli.application import Application
-    from ddev.cli.validate.all.orchestrator import ValidationResult
+    from ddev.cli.validate.all.orchestrator import ValidationConfig, ValidationResult
 
 COMMENT_HEADING = "## Validation Report"
 
@@ -75,27 +75,112 @@ def write_step_summary(content: str) -> None:
                 f.write(content + "\n")
 
 
+def _build_preamble(error: str | None, warning: str | None) -> list[str]:
+    parts: list[str] = [f"{COMMENT_HEADING}\n"]
+    if error:
+        parts.append(f"> **Error:** {error}\n")
+    if warning:
+        parts.append(f"> **Warning:** {warning}\n")
+    return parts
+
+
+def _build_table(
+    rows: dict[str, ValidationResult],
+    configs: dict[str, ValidationConfig],
+) -> list[str]:
+    """Build a markdown table with Validation, Description, and Status columns."""
+    from ddev.cli.validate.all.orchestrator import ValidationConfig as _VC
+
+    lines = ["| Validation | Description | Status |", "|---|---|---|"]
+    for name in sorted(rows):
+        status = "✅" if rows[name].success else "❌"
+        description = configs.get(name, _VC()).description
+        lines.append(f"| `{name}` | {description} | {status} |")
+    return lines
+
+
+def _build_report_section(
+    rows: dict[str, ValidationResult],
+    configs: dict[str, ValidationConfig],
+    *,
+    header: str | None = None,
+    collapsed: bool = False,
+) -> list[str]:
+    table = _build_table(rows, configs)
+    if collapsed:
+        summary = header or "Details"
+        return [
+            "",
+            f"<details>\n<summary>{summary}</summary>\n",
+            *table,
+            "\n</details>",
+        ]
+    if header:
+        return [header, "", *table]
+    return table
+
+
+def _build_incomplete_warning(expected_validations: list[str], results: dict[str, ValidationResult]) -> list[str]:
+    missing = sorted(set(expected_validations) - set(results))
+    if not missing:
+        return []
+    names = ", ".join(f"`{n}`" for n in missing)
+    return [f"> **Warning:** {len(missing)} validation(s) did not complete: {names}\n"]
+
+
 def format_pr_comment(
     results: dict[str, ValidationResult],
+    configs: dict[str, ValidationConfig],
     target: str | None,
+    expected_validations: list[str],
     *,
     error: str | None = None,
     warning: str | None = None,
 ) -> str:
-    failures = {n for n, r in results.items() if not r.success}
-
-    parts = [f"{COMMENT_HEADING}\n"]
-    if error:
-        parts.append(f"> **Error:** {error}\n")
-    if warning:
-        parts.append(f"> **Warning:** {warning}\n")
+    """Format a PR comment with collapsible sections to reduce clutter."""
+    failures: dict[str, ValidationResult] = {}
+    passed: dict[str, ValidationResult] = {}
+    for name, result in results.items():
+        (passed if result.success else failures)[name] = result
 
-    parts.extend(("| Validation | Status |", "|---|---|"))
-    for name in sorted(results):
-        status = "❌" if name in failures else "✅"
-        parts.append(f"| `{name}` | {status} |")
+    incomplete = _build_incomplete_warning(expected_validations, results)
+    parts = _build_preamble(error, warning)
+    parts.extend(incomplete)
 
     if failures:
+        parts.extend(_build_report_section(failures, configs))
+        fix_target = f" {target}" if target else ""
+        parts.append(f"\nRun `ddev validate all{fix_target} --fix` to attempt to auto-fix supported validations.")
+
+    if passed:
+        if failures or incomplete:
+            header = f"Passed validations ({len(passed)})"
+        else:
+            parts.append(f"All {len(passed)} validations passed.")
+            header = "Show details"
+        parts.extend(_build_report_section(passed, configs, header=header, collapsed=True))
+
+    return "\n".join(parts)
+
+
+def format_step_summary(
+    results: dict[str, ValidationResult],
+    configs: dict[str, ValidationConfig],
+    target: str | None,
+    expected_validations: list[str],
+    *,
+    error: str | None = None,
+    warning: str | None = None,
+) -> str:
+    """Format a flat summary table for the GitHub Actions step summary."""
+    has_failures = any(not r.success for r in results.values())
+
+    parts = _build_preamble(error, warning)
+    parts.extend(_build_incomplete_warning(expected_validations, results))
+
+    parts.extend(_build_table(results, configs))
+
+    if has_failures:
         fix_target = f" {target}" if target else ""
         fix_all_cmd = f"ddev validate all{fix_target} --fix"
         parts.append(f"\nRun `{fix_all_cmd}` to attempt to auto-fix supported validations.")

ddev/src/ddev/cli/validate/all/orchestrator.py

@@ -16,6 +16,7 @@
 from ddev.cli.validate.all.github import (
     COMMENT_HEADING,
     format_pr_comment,
+    format_step_summary,
     get_workflow_run_url,
     write_step_summary,
 )
@@ -27,6 +28,7 @@
 
 @dataclass(frozen=True)
 class ValidationConfig:
+    description: str = ""
     repo_wide: bool = False
     fix_flag: str | None = None
 
@@ -36,26 +38,78 @@ class ValidationConfig:
 # are migrated, we can improve the command structure so each validation
 # auto-registers itself instead of maintaining this manually.
 VALIDATIONS: dict[str, ValidationConfig] = {
-    "agent-reqs": ValidationConfig(),
-    "ci": ValidationConfig(repo_wide=True, fix_flag="--sync"),
-    "codeowners": ValidationConfig(repo_wide=True),
-    "config": ValidationConfig(fix_flag="--sync"),
-    "dep": ValidationConfig(repo_wide=True),
-    "http": ValidationConfig(),
-    "imports": ValidationConfig(),
-    "integration-style": ValidationConfig(),
-    "jmx-metrics": ValidationConfig(),
-    "labeler": ValidationConfig(repo_wide=True, fix_flag="--sync"),
-    "legacy-signature": ValidationConfig(),
-    "license-headers": ValidationConfig(fix_flag="--fix"),
-    "licenses": ValidationConfig(repo_wide=True, fix_flag="--sync"),
-    "metadata": ValidationConfig(fix_flag="--sync"),
-    "models": ValidationConfig(fix_flag="--sync"),
-    "openmetrics": ValidationConfig(),
-    "package": ValidationConfig(),
-    "readmes": ValidationConfig(),
-    "saved-views": ValidationConfig(),
-    "version": ValidationConfig(),
+    "agent-reqs": ValidationConfig(
+        description="Verify check versions match the Agent requirements file",
+    ),
+    "ci": ValidationConfig(
+        description="Validate CI configuration and Codecov settings",
+        repo_wide=True,
+        fix_flag="--sync",
+    ),
+    "codeowners": ValidationConfig(
+        description="Validate every integration has a CODEOWNERS entry",
+        repo_wide=True,
+    ),
+    "config": ValidationConfig(
+        description="Validate default configuration files against spec.yaml",
+        fix_flag="--sync",
+    ),
+    "dep": ValidationConfig(
+        description="Verify dependency pins are consistent and Agent-compatible",
+        repo_wide=True,
+    ),
+    "http": ValidationConfig(
+        description="Validate integrations use the HTTP wrapper correctly",
+    ),
+    "imports": ValidationConfig(
+        description="Validate check imports do not use deprecated modules",
+    ),
+    "integration-style": ValidationConfig(
+        description="Validate check code style conventions",
+    ),
+    "jmx-metrics": ValidationConfig(
+        description="Validate JMX metrics definition files and config",
+    ),
+    "labeler": ValidationConfig(
+        description="Validate PR labeler config matches integration directories",
+        repo_wide=True,
+        fix_flag="--sync",
+    ),
+    "legacy-signature": ValidationConfig(
+        description="Validate no integration uses the legacy Agent check signature",
+    ),
+    "license-headers": ValidationConfig(
+        description="Validate Python files have proper license headers",
+        fix_flag="--fix",
+    ),
+    "licenses": ValidationConfig(
+        description="Validate third-party license attribution list",
+        repo_wide=True,
+        fix_flag="--sync",
+    ),
+    "metadata": ValidationConfig(
+        description="Validate metadata.csv metric definitions",
+        fix_flag="--sync",
+    ),
+    "models": ValidationConfig(
+        description="Validate configuration data models match spec.yaml",
+        fix_flag="--sync",
+    ),
+    "openmetrics": ValidationConfig(
+        description="Validate OpenMetrics integrations disable the metric limit",
+    ),
+    "package": ValidationConfig(
+        description="Validate Python package metadata and naming",
+    ),
+    "readmes": ValidationConfig(
+        description="Validate README files have required sections",
+    ),
+    "saved-views": ValidationConfig(
+        description="Validate saved view JSON file structure and fields",
+    ),
+    "version": ValidationConfig(
+        description="Validate version consistency between package and changelog",
+    ),
 }
 
 
@@ -166,21 +220,18 @@ async def on_finalize(self, exception: Exception | None) -> None:
         if exception is not None:
             self._app.display_error(f"Error running validations: {exception}")
 
-        self._post_pr_comment(exception)
+        self._publish_report(exception)
         self._print_console_output()
 
-    def _build_report_body(self, exception: Exception | None) -> str:
+    def _build_error_and_warning(self, exception: Exception | None) -> tuple[str | None, str | None]:
         error_msg = f"Error running validations: {exception}" if exception else None
 
         if self._pr_number is None and os.environ.get("GITHUB_EVENT_NAME") == "pull_request":
             extra_warning = "Running in pull_request context but could not determine PR number to post a comment."
         else:
             extra_warning = None
 
-        body = format_pr_comment(self._results, self._target, error=error_msg, warning=extra_warning)
-        if run_url := get_workflow_run_url():
-            body += f"\n\n[View full run]({run_url})"
-        return body
+        return error_msg, extra_warning
 
     def _delete_previous_comments(self, pr_number: int) -> None:
         try:
@@ -191,9 +242,29 @@ def _delete_previous_comments(self, pr_number: int) -> None:
         except Exception as exc:
             self._app.display_warning(f"Failed to clean up previous validation comments: {exc}")
 
-    def _post_pr_comment(self, exception: Exception | None) -> None:
-        body = self._build_report_body(exception)
-        write_step_summary(body)
+    def _publish_report(self, exception: Exception | None) -> None:
+        error_msg, extra_warning = self._build_error_and_warning(exception)
+
+        summary_body = format_step_summary(
+            self._results,
+            VALIDATIONS,
+            self._target,
+            self._validations,
+            error=error_msg,
+            warning=extra_warning,
+        )
+        write_step_summary(summary_body)
+
+        comment_body = format_pr_comment(
+            self._results,
+            VALIDATIONS,
+            self._target,
+            self._validations,
+            error=error_msg,
+            warning=extra_warning,
+        )
+        if run_url := get_workflow_run_url():
+            comment_body += f"\n\n[View full run]({run_url})"
 
         self._app.logger.debug("PR number: %s", self._pr_number)
         self._app.logger.debug("GitHub token configured: %s", bool(self._app.config.github.token))
@@ -212,11 +283,11 @@ def _post_pr_comment(self, exception: Exception | None) -> None:
             self._app.logger.debug("Deleting previous validation comments on PR #%s...", self._pr_number)
             self._delete_previous_comments(self._pr_number)
             self._app.logger.debug("Posting validation comment on PR #%s...", self._pr_number)
-            self._app.github.post_pull_request_comment(self._pr_number, body)
+            self._app.github.post_pull_request_comment(self._pr_number, comment_body)
             self._app.logger.debug("Comment posted successfully.")
         except Exception as exc:
-            self._app.display_warning(f"Failed to post PR comment: {exc}")
-            write_step_summary(f"\n> Failed to post PR comment: {exc}")
+            self._app.display_warning(f"Failed to post PR comment: {type(exc).__name__}: {exc}")
+            write_step_summary(f"\n> Failed to post PR comment: {type(exc).__name__}: {exc}")
         finally:
             httpx_logger.setLevel(previous_level)