Skip to content

feat: markdown-formatted review summaries with per-finding list items#97

Merged
waynesun09 merged 1 commit into
mainfrom
fix-review-summary-format
May 22, 2026
Merged

feat: markdown-formatted review summaries with per-finding list items#97
waynesun09 merged 1 commit into
mainfrom
fix-review-summary-format

Conversation

@waynesun09

@waynesun09 waynesun09 commented May 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • Replace semicolon-joined one-liner format in _unpack_bare_array with markdown list items (## severity headings, - bullets per finding, fenced code blocks for snippets)
  • Update AI summarization prompts (_SUMMARY_RULES, _RESPONSE_FORMAT) to guide models toward markdown formatting — matches what gemini-3.5-flash naturally produces
  • Add _format_finding_md helper for consistent per-finding formatting across fallback and AI paths

Test plan

  • All 85 summarizer tests pass including 3 new tests (special char preservation, code snippets, suggestions)
  • Full test suite passes (1402 passed, 2 skipped)
  • Pre-commit checks pass
  • E2E verified against GitHub PR feat: markdown-formatted review summaries with per-finding list items #97 (4 severity headings, 7 findings, 4 code blocks, 5 suggestions)
  • E2E verified against GitLab MR !152 (3 severity headings, 7 findings, 7 suggestions)
  • Markdown renders correctly in both GitHub PR and GitLab MR comments

@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR refactors the finding summarizer to enforce stricter Markdown formatting rules and reorganize how individual findings are rendered. The AI instruction rules are tightened to require severity headings with counts, one-per-bullet formatting, backticked file paths, and fenced code blocks. A new _format_finding_md helper renders each finding individually, and the summary builder is reworked to use this formatter. Tests are updated to match the new structure with new cases for special characters, code snippets, and suggestions.

Changes

Finding Summarization Markdown Formatting

Layer / File(s) Summary
Markdown formatting rules
src/cicaddy/delegation/summarizer.py
AI instruction text updated to require severity headings with per-severity counts, one-finding-per-bullet structure, backticked file paths, and fenced code blocks for code snippets.
Finding formatter helper
src/cicaddy/delegation/summarizer.py
New _format_finding_md(entry) function converts a finding dictionary into a Markdown bullet body that conditionally includes file/message text, an optional fenced existing_code snippet, and an optional italicized suggestion line.
Summary builder refactor
src/cicaddy/delegation/summarizer.py
The bare-array unpacking summary builder is reworked to group full finding dictionaries by severity and render each as its own bullet using _format_finding_md, replacing previous message-only grouping and inline concatenation.
Test coverage for new formatting
tests/unit/test_delegation_summarizer.py
Existing assertions updated to expect counted severity headers and per-file bullet formatting; three new tests verify special character preservation, fenced code block rendering for existing_code, and italicized suggestion formatting.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • waynesun09/cicaddy#52: Introduced the summarizer module that this PR refactors with stricter Markdown formatting and a new finding formatter helper.
  • waynesun09/cicaddy#92: Previously modified the same _unpack_bare_array and summary rendering logic; this PR further refines the Markdown bullet and severity formatting.
  • waynesun09/cicaddy#54: Expanded the finding schema to include existing_code and suggestion fields; this PR implements the Markdown rendering for those new fields.

Poem

🐰 Hop, hop—findings now dance in bullet form,
With fences for code and suggestions transformed,
Each severity counted, each message so clear,
The summarizer's Markdown is finally here!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title states 'markdown-formatted review summaries' but the PR objectives indicate the actual implementation uses HTML-formatted output with

,
  • , and
     tags, not Markdown.

Update the PR title to accurately reflect HTML formatting, e.g., 'feat: HTML-formatted review summaries with per-finding list items' to match the actual implementation.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-review-summary-format

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

…t items

Replace semicolon-joined one-liner format in bare array fallback with
proper markdown: ## headings per severity group, bullet list with one
finding per line, backtick-wrapped file paths, fenced code blocks for
snippets, and italic suggestions.

Update AI summarization prompts to explicitly instruct markdown
formatting with one-finding-per-bullet structure, matching the natural
output format of gemini-3.5-flash.

Signed-off-by: Wayne Sun <gsun@redhat.com>
@waynesun09
waynesun09 force-pushed the fix-review-summary-format branch from 042d517 to 9a4d6f7 Compare May 22, 2026 20:40
@sonarqubecloud

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/cicaddy/delegation/summarizer.py (1)

500-523: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Unknown severities can disappear from the generated summary.

Line 500 groups any severity value, but Lines 508-517 only render critical|major|minor|nit. If a finding has an unexpected severity, it is counted in the intro total but omitted from all bullet sections.

Proposed fix
-        for f in findings:
-            sev = str(f.get("severity", "minor")).lower()
+        for f in findings:
+            sev = str(f.get("severity", "minor")).lower()
+            if sev not in {"critical", "major", "minor", "nit"}:
+                sev = "minor"
             msg = f.get("message", "")
             if msg:
                 severity_groups.setdefault(sev, []).append(f)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cicaddy/delegation/summarizer.py` around lines 500 - 523, The summary
currently collects all findings into severity_groups but only renders sections
for ("critical","major","minor","nit"), causing unknown severities to be counted
in total but not displayed; update the rendering logic in summarizer.py so after
building severity_groups you compute an ordered list that first includes the
known severities and then any remaining keys (e.g. remaining = sorted(k for k in
severity_groups.keys() if k not in known)), then iterate that combined list to
append sections using _format_finding_md for each entry; ensure you still
compute total from severity_groups and include unknown severity names (or a
generic "Other" if you prefer) in the section headers so no findings are
omitted.
🧹 Nitpick comments (1)
src/cicaddy/delegation/summarizer.py (1)

126-127: ⚡ Quick win

Use a Google-style docstring for the new helper.

Please update this new function docstring to Google style (Args/Returns) to match repository standards for src/**/*.py.

As per coding guidelines, "Use type hints and Google-style docstrings in Python code".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cicaddy/delegation/summarizer.py` around lines 126 - 127, Update the
docstring for the helper function _format_finding_md to use Google-style
formatting: start with a one-line summary, then an Args section documenting
entry (dict) and its expected contents, and a Returns section describing the
returned str (markdown bullet body). Ensure type hints match (entry: dict) and
keep the description concise to match repository standards.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/cicaddy/delegation/summarizer.py`:
- Around line 500-523: The summary currently collects all findings into
severity_groups but only renders sections for
("critical","major","minor","nit"), causing unknown severities to be counted in
total but not displayed; update the rendering logic in summarizer.py so after
building severity_groups you compute an ordered list that first includes the
known severities and then any remaining keys (e.g. remaining = sorted(k for k in
severity_groups.keys() if k not in known)), then iterate that combined list to
append sections using _format_finding_md for each entry; ensure you still
compute total from severity_groups and include unknown severity names (or a
generic "Other" if you prefer) in the section headers so no findings are
omitted.

---

Nitpick comments:
In `@src/cicaddy/delegation/summarizer.py`:
- Around line 126-127: Update the docstring for the helper function
_format_finding_md to use Google-style formatting: start with a one-line
summary, then an Args section documenting entry (dict) and its expected
contents, and a Returns section describing the returned str (markdown bullet
body). Ensure type hints match (entry: dict) and keep the description concise to
match repository standards.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b86df22a-ba37-4148-a4d8-4c5c498f3cf9

📥 Commits

Reviewing files that changed from the base of the PR and between 4b08b59 and 9a4d6f7.

📒 Files selected for processing (2)
  • src/cicaddy/delegation/summarizer.py
  • tests/unit/test_delegation_summarizer.py

@waynesun09 waynesun09 changed the title feat: HTML-formatted review summaries with per-finding list items feat: markdown-formatted review summaries with per-finding list items May 22, 2026
@waynesun09
waynesun09 merged commit 7bcc353 into main May 22, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant