From 5d2cee307b64a212d4ced58a1e3df3c07fb5a092 Mon Sep 17 00:00:00 2001 From: Dimitris Kargatzis Date: Fri, 27 Feb 2026 11:55:19 +0200 Subject: [PATCH 1/7] ux: refactor PR comments and Check Run formatting to use collapsible details and actionable footers --- src/presentation/github_formatter.py | 34 ++++++++++++------- .../presentation/test_github_formatter.py | 19 ++++++----- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/presentation/github_formatter.py b/src/presentation/github_formatter.py index 10ad350..e0895bd 100644 --- a/src/presentation/github_formatter.py +++ b/src/presentation/github_formatter.py @@ -103,18 +103,22 @@ def format_check_run_output( for severity in severity_order: if severity_groups[severity]: emoji = SEVERITY_STR_EMOJI.get(severity, "⚪") - text += f"## {emoji} {severity.title()} Severity\n\n" + 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\n" - text += f"{violation.message}\n\n" - text += f"Rule validation failed with severity: **{violation.severity}**\n" + 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" 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} @@ -152,7 +156,10 @@ 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" + if not violations: + return "" + + comment = f"### 🛡️ Watchflow Governance Checks\n**Status:** ❌ {len(violations)} Violations Found\n\n" # Group violations by severity severity_order = ["critical", "high", "medium", "low"] @@ -167,19 +174,22 @@ def format_violations_comment(violations: list[Violation]) -> str: for severity in severity_order: if severity_groups[severity]: emoji = SEVERITY_STR_EMOJI.get(severity, "⚪") - comment += f"### {emoji} {severity.title()} Severity\n\n" + count = len(severity_groups[severity]) + + comment += "
\n" + comment += f"{emoji} {severity.title()} Severity ({count})\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" + comment += f"**{violation.rule_description or 'Unknown Rule'}**\n" + comment += f"{violation.message}\n" if violation.how_to_fix: comment += f"**How to fix:** {violation.how_to_fix}\n" comment += "\n" + + comment += "
\n\n" 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 diff --git a/tests/unit/presentation/test_github_formatter.py b/tests/unit/presentation/test_github_formatter.py index 5d10f30..8aca4b8 100644 --- a/tests/unit/presentation/test_github_formatter.py +++ b/tests/unit/presentation/test_github_formatter.py @@ -16,20 +16,21 @@ 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 "### Rule 2" in comment - assert "### Rule 1" in comment - assert "### Rule 3" 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_format_check_run_output_success(): @@ -46,7 +47,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"] From c0a156ee038d220c96b3a4bbcf8adff7634776a0 Mon Sep 17 00:00:00 2001 From: Dimitris Kargatzis Date: Fri, 27 Feb 2026 12:01:43 +0200 Subject: [PATCH 2/7] style: fix trailing whitespace pre-commit issues --- src/presentation/github_formatter.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/presentation/github_formatter.py b/src/presentation/github_formatter.py index e0895bd..297f3ed 100644 --- a/src/presentation/github_formatter.py +++ b/src/presentation/github_formatter.py @@ -104,7 +104,7 @@ def format_check_run_output( 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" @@ -114,7 +114,7 @@ def format_check_run_output( if violation.how_to_fix: text += f"**How to fix:** {violation.how_to_fix}\n" text += "\n" - + text += "
\n\n" text += "---\n" @@ -175,7 +175,7 @@ def format_violations_comment(violations: list[Violation]) -> str: if severity_groups[severity]: emoji = SEVERITY_STR_EMOJI.get(severity, "⚪") count = len(severity_groups[severity]) - + comment += "
\n" comment += f"{emoji} {severity.title()} Severity ({count})\n\n" @@ -185,7 +185,7 @@ def format_violations_comment(violations: list[Violation]) -> str: if violation.how_to_fix: comment += f"**How to fix:** {violation.how_to_fix}\n" comment += "\n" - + comment += "
\n\n" comment += "---\n" From efedc65ca5f0b748bedf950d60258bf224d5ecc4 Mon Sep 17 00:00:00 2001 From: Dimitris Kargatzis Date: Fri, 27 Feb 2026 12:32:29 +0200 Subject: [PATCH 3/7] ux: apply collapsible details format to PR check runs and acknowledgment summaries --- src/presentation/github_formatter.py | 87 +++++++++---------- .../presentation/test_github_formatter.py | 6 +- 2 files changed, 43 insertions(+), 50 deletions(-) diff --git a/src/presentation/github_formatter.py b/src/presentation/github_formatter.py index 297f3ed..f586c4a 100644 --- a/src/presentation/github_formatter.py +++ b/src/presentation/github_formatter.py @@ -23,6 +23,43 @@ } +def _build_collapsible_violations_text(violations: list[Violation]) -> str: + """Builds a collapsible Markdown string grouped by severity for a list of violations.""" + if not violations: + return "" + + text = "" + severity_order = ["critical", "high", "medium", "low"] + 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: + if "low" not in severity_groups: + severity_groups["low"] = [] + 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, @@ -99,24 +136,7 @@ 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, "⚪") - 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" - + text += _build_collapsible_violations_text(violations) text += "---\n" text += "💡 *To configure rules, edit the `.watchflow/rules.yaml` file in this repository.*" @@ -160,34 +180,7 @@ def format_violations_comment(violations: list[Violation]) -> str: return "" comment = f"### 🛡️ Watchflow Governance Checks\n**Status:** ❌ {len(violations)} Violations Found\n\n" - - # Group violations by severity - severity_order = ["critical", "high", "medium", "low"] - 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) - - # Add violations by severity (most severe first) - for severity in severity_order: - if severity_groups[severity]: - emoji = SEVERITY_STR_EMOJI.get(severity, "⚪") - count = len(severity_groups[severity]) - - comment += "
\n" - comment += f"{emoji} {severity.title()} Severity ({count})\n\n" - - for violation in severity_groups[severity]: - comment += f"**{violation.rule_description or 'Unknown Rule'}**\n" - comment += f"{violation.message}\n" - if violation.how_to_fix: - comment += f"**How to fix:** {violation.how_to_fix}\n" - comment += "\n" - - comment += "
\n\n" - + comment += _build_collapsible_violations_text(violations) comment += "---\n" comment += "💡 *Reply with `@watchflow ack [reason]` to override these rules, or `@watchflow help` for commands.*" @@ -271,7 +264,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 8aca4b8..db965d1 100644 --- a/tests/unit/presentation/test_github_formatter.py +++ b/tests/unit/presentation/test_github_formatter.py @@ -20,9 +20,9 @@ def test_format_violations_comment_groups_by_severity(): 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 "### 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 From ecaacddc411a2ae01dfcba8e14cf014560059894 Mon Sep 17 00:00:00 2001 From: Dimitris Kargatzis Date: Fri, 27 Feb 2026 13:02:20 +0200 Subject: [PATCH 4/7] style: fix remaining trailing whitespace from presentation formatter --- src/presentation/github_formatter.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/presentation/github_formatter.py b/src/presentation/github_formatter.py index f586c4a..b5a9c06 100644 --- a/src/presentation/github_formatter.py +++ b/src/presentation/github_formatter.py @@ -45,7 +45,7 @@ def _build_collapsible_violations_text(violations: list[Violation]) -> str: 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" @@ -55,11 +55,12 @@ def _build_collapsible_violations_text(violations: list[Violation]) -> str: 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, From 7bd737b6e21b18b92442c197018d20d6085b300a Mon Sep 17 00:00:00 2001 From: Dimitris Kargatzis Date: Fri, 27 Feb 2026 13:13:22 +0200 Subject: [PATCH 5/7] ux: align PR recommendation body formatting with the new CodeRabbit-style collapsible sections --- src/api/recommendations.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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( From 29458725e14ec6eede417fded0c053a8237bd894 Mon Sep 17 00:00:00 2001 From: Dimitris Kargatzis Date: Fri, 27 Feb 2026 20:10:39 +0200 Subject: [PATCH 6/7] test: add test coverage for missing severity fallback logic in formatter --- src/presentation/github_formatter.py | 31 +++++++++++++++++-- .../presentation/test_github_formatter.py | 13 ++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/presentation/github_formatter.py b/src/presentation/github_formatter.py index b5a9c06..67392f1 100644 --- a/src/presentation/github_formatter.py +++ b/src/presentation/github_formatter.py @@ -24,7 +24,14 @@ def _build_collapsible_violations_text(violations: list[Violation]) -> str: - """Builds a collapsible Markdown string grouped by severity for a list of violations.""" + """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 "" @@ -67,7 +74,17 @@ def format_check_run_output( 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(): @@ -176,7 +193,15 @@ def format_rules_not_configured_comment( def format_violations_comment(violations: list[Violation]) -> str: - """Format violations as a GitHub comment.""" + """Format violations as a GitHub comment. + + Args: + violations: List of rule violations to include in the comment. + + 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 "" diff --git a/tests/unit/presentation/test_github_formatter.py b/tests/unit/presentation/test_github_formatter.py index db965d1..636d317 100644 --- a/tests/unit/presentation/test_github_formatter.py +++ b/tests/unit/presentation/test_github_formatter.py @@ -33,6 +33,19 @@ def test_format_violations_comment_empty(): 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_format_check_run_output_success(): output = format_check_run_output([]) assert output["title"] == "All rules passed" From 33fa29c1f40254c27fb3e6b733d536fb32c97c7a Mon Sep 17 00:00:00 2001 From: Dimitris Kargatzis Date: Fri, 27 Feb 2026 20:22:14 +0200 Subject: [PATCH 7/7] fix: ensure INFO severity rules are properly grouped instead of falling back to LOW --- src/presentation/github_formatter.py | 6 ++---- tests/unit/presentation/test_github_formatter.py | 9 +++++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/presentation/github_formatter.py b/src/presentation/github_formatter.py index 67392f1..e5ed98e 100644 --- a/src/presentation/github_formatter.py +++ b/src/presentation/github_formatter.py @@ -36,7 +36,7 @@ def _build_collapsible_violations_text(violations: list[Violation]) -> str: return "" text = "" - 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: @@ -44,8 +44,6 @@ def _build_collapsible_violations_text(violations: list[Violation]) -> str: if sev in severity_groups: severity_groups[sev].append(violation) else: - if "low" not in severity_groups: - severity_groups["low"] = [] severity_groups["low"].append(violation) for severity in severity_order: @@ -130,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: diff --git a/tests/unit/presentation/test_github_formatter.py b/tests/unit/presentation/test_github_formatter.py index 636d317..a4c6393 100644 --- a/tests/unit/presentation/test_github_formatter.py +++ b/tests/unit/presentation/test_github_formatter.py @@ -46,6 +46,15 @@ def test_build_collapsible_violations_text_fallback_severity(): 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(): output = format_check_run_output([]) assert output["title"] == "All rules passed"