diff --git a/src/api/recommendations.py b/src/api/recommendations.py index 49f39bc..35d30c0 100644 --- a/src/api/recommendations.py +++ b/src/api/recommendations.py @@ -351,16 +351,19 @@ def generate_pr_body( body_lines.extend( [ - f"### {description} - {severity}", + "
", + f"{severity} Severity: {description}", + "", ] ) if reasoning: body_lines.extend( [ - "", f"**Rationale:** {reasoning}", + "", ] ) + body_lines.append("
") body_lines.append("") body_lines.extend( diff --git a/src/presentation/github_formatter.py b/src/presentation/github_formatter.py index 10ad350..e5ed98e 100644 --- a/src/presentation/github_formatter.py +++ b/src/presentation/github_formatter.py @@ -23,13 +23,66 @@ } +def _build_collapsible_violations_text(violations: list[Violation]) -> str: + """Builds a collapsible Markdown string grouped by severity for a list of violations. + + Args: + violations: A list of Violation objects to format. + + Returns: + A Markdown formatted string with collapsible details blocks for each severity level. + """ + if not violations: + return "" + + text = "" + severity_order = ["critical", "high", "medium", "low", "info"] + severity_groups: dict[str, list[Violation]] = {s: [] for s in severity_order} + + for violation in violations: + sev = violation.severity.value if hasattr(violation.severity, "value") else str(violation.severity) + if sev in severity_groups: + severity_groups[sev].append(violation) + else: + severity_groups["low"].append(violation) + + for severity in severity_order: + if severity_groups[severity]: + emoji = SEVERITY_STR_EMOJI.get(severity, "⚪") + count = len(severity_groups[severity]) + + text += "
\n" + text += f"{emoji} {severity.title()} Severity ({count})\n\n" + + for violation in severity_groups[severity]: + text += f"### {violation.rule_description or 'Unknown Rule'}\n" + text += f"{violation.message}\n" + if violation.how_to_fix: + text += f"**How to fix:** {violation.how_to_fix}\n" + text += "\n" + + text += "
\n\n" + + return text + + def format_check_run_output( violations: list[Violation], error: str | None = None, repo_full_name: str | None = None, installation_id: int | None = None, ) -> dict[str, Any]: - """Format violations for check run output.""" + """Format violations for check run output. + + Args: + violations: List of rule violations to report. + error: Optional error message if rule processing failed entirely. + repo_full_name: The full repository name (e.g., owner/repo). + installation_id: The GitHub App installation ID. + + Returns: + A dictionary matching the GitHub Check Run Output schema containing title, summary, and text. + """ if error: # Check if it's a missing rules file error if "rules not configured" in error.lower() or "rules file not found" in error.lower(): @@ -75,7 +128,7 @@ def format_check_run_output( } # Group violations by severity - severity_order = ["critical", "high", "medium", "low"] + severity_order = ["critical", "high", "medium", "low", "info"] severity_groups: dict[str, list[Violation]] = {s: [] for s in severity_order} for violation in violations: @@ -99,22 +152,9 @@ def format_check_run_output( # Build detailed text text = "# Watchflow Rule Violations\n\n" - - for severity in severity_order: - if severity_groups[severity]: - emoji = SEVERITY_STR_EMOJI.get(severity, "⚪") - text += f"## {emoji} {severity.title()} Severity\n\n" - - for violation in severity_groups[severity]: - text += f"### {violation.rule_description or 'Unknown Rule'}\n\n" - text += f"{violation.message}\n\n" - text += f"Rule validation failed with severity: **{violation.severity}**\n" - if violation.how_to_fix: - text += f"**How to fix:** {violation.how_to_fix}\n" - text += "\n" - + text += _build_collapsible_violations_text(violations) text += "---\n" - text += "*To configure rules, edit the `.watchflow/rules.yaml` file in this repository.*" + text += "💡 *To configure rules, edit the `.watchflow/rules.yaml` file in this repository.*" return {"title": f"{len(violations)} rule violations found", "summary": summary, "text": text} @@ -151,35 +191,22 @@ def format_rules_not_configured_comment( def format_violations_comment(violations: list[Violation]) -> str: - """Format violations as a GitHub comment.""" - comment = "## 🚨 Watchflow Rule Violations Detected\n\n" - - # Group violations by severity - severity_order = ["critical", "high", "medium", "low"] - severity_groups: dict[str, list[Violation]] = {s: [] for s in severity_order} + """Format violations as a GitHub comment. - for violation in violations: - sev = violation.severity.value if hasattr(violation.severity, "value") else str(violation.severity) - if sev in severity_groups: - severity_groups[sev].append(violation) + Args: + violations: List of rule violations to include in the comment. - # Add violations by severity (most severe first) - for severity in severity_order: - if severity_groups[severity]: - emoji = SEVERITY_STR_EMOJI.get(severity, "⚪") - comment += f"### {emoji} {severity.title()} Severity\n\n" - - for violation in severity_groups[severity]: - comment += f"### {violation.rule_description or 'Unknown Rule'}\n\n" - comment += f"{violation.message}\n\n" - comment += f"Rule validation failed with severity: **{violation.severity}**\n" - if violation.how_to_fix: - comment += f"**How to fix:** {violation.how_to_fix}\n" - comment += "\n" + Returns: + A Markdown formatted string suitable for a Pull Request timeline comment. + Returns an empty string if there are no violations. + """ + if not violations: + return "" + comment = f"### 🛡️ Watchflow Governance Checks\n**Status:** ❌ {len(violations)} Violations Found\n\n" + comment += _build_collapsible_violations_text(violations) comment += "---\n" - comment += "*This comment was automatically generated by [Watchflow](https://watchflow.dev).*\n" - comment += "*To configure rules, edit the `.watchflow/rules.yaml` file in this repository.*" + comment += "💡 *Reply with `@watchflow ack [reason]` to override these rules, or `@watchflow help` for commands.*" return comment @@ -261,7 +288,7 @@ def format_acknowledgment_check_run( {format_acknowledgment_summary(acknowledgable_violations, acknowledgments)} **Violations Requiring Fixes:** -{format_violations_for_check_run(violations)} +{_build_collapsible_violations_text(violations)} Please address the remaining violations or acknowledge them with a valid reason. """ diff --git a/tests/unit/presentation/test_github_formatter.py b/tests/unit/presentation/test_github_formatter.py index 5d10f30..a4c6393 100644 --- a/tests/unit/presentation/test_github_formatter.py +++ b/tests/unit/presentation/test_github_formatter.py @@ -16,20 +16,43 @@ def test_format_violations_comment_groups_by_severity(): comment = format_violations_comment(violations) - assert "## 🚨 Watchflow Rule Violations Detected" in comment - assert "### 🔴 Critical Severity" in comment - assert "### 🟠 High Severity" in comment + assert "### 🛡️ Watchflow Governance Checks" in comment + assert "**Status:** ❌ 3 Violations Found" in comment + assert "🔴 Critical Severity (1)" in comment + assert "🟠 High Severity (2)" in comment assert "### Rule 2" in comment assert "### Rule 1" in comment assert "### Rule 3" in comment assert "Fix 1" in comment assert "Fix 2" in comment + assert "💡 *Reply with `@watchflow ack [reason]`" in comment def test_format_violations_comment_empty(): comment = format_violations_comment([]) - assert "## 🚨 Watchflow Rule Violations Detected" in comment - assert "---" in comment + assert comment == "" + + +def test_build_collapsible_violations_text_fallback_severity(): + """Test that a violation with an unknown severity falls back to 'low' severity bucket.""" + # Create a violation and use object.__setattr__ to bypass Pydantic validation for testing + v = Violation(rule_description="Weird Rule", severity=Severity.LOW, message="Weird error") + object.__setattr__(v, "severity", "super_critical_unknown") + + comment = format_violations_comment([v]) + + # It should fall back to low severity + assert "🟢 Low Severity (1)" in comment + assert "Weird error" in comment + + +def test_build_collapsible_violations_text_info_severity(): + """Test that INFO severity violations are correctly formatted.""" + v = Violation(rule_description="Info Rule", severity=Severity.INFO, message="Just an info message") + comment = format_violations_comment([v]) + + assert "⚪ Info Severity (1)" in comment + assert "Just an info message" in comment def test_format_check_run_output_success(): @@ -46,7 +69,7 @@ def test_format_check_run_output_with_violations(): assert "1 rule violations found" in output["title"] assert "🚨 1 violations found: 1 high" in output["summary"] - assert "## 🟠 High Severity" in output["text"] + assert "🟠 High Severity (1)" in output["text"] assert "### Missing Issue" in output["text"]