NO-JIRA: feat(ci-tooling): add HTML dashboard report to pr-report skill#8558
NO-JIRA: feat(ci-tooling): add HTML dashboard report to pr-report skill#8558jparrill wants to merge 1 commit into
Conversation
The weekly PR report script now generates a self-contained HTML dashboard alongside the existing markdown and JSON outputs. The HTML includes stat cards, repo breakdown chart, merge activity, top reviewers, and categorized PR tables with a dark theme. Highlights are injected by Claude as a post-processing step after generating the impact analysis report, keeping the script output deterministic while adding semantic context to the dashboard. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Skipping CI for Draft Pull Request. |
|
@jparrill: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThis PR adds HTML dashboard report generation to the weekly PR report tool. The changes include documentation updates specifying the new 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jparrill The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Sample OutputLive HTML preview: https://gist.github.com/jparrill/3bace3328b6d6d6b8206a07c7fa426a2 To view the rendered dashboard, download the HTML file from the gist and open it locally, or use htmlpreview.github.io. The HTML dashboard includes:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@contrib/repo_metrics/weekly_pr_report.py`:
- Around line 1217-1222: The code calls min(self.prs, ...) to compute `fastest`
and then formats `fastest_h`, which raises ValueError when `self.prs` (and thus
`merge_times`) is empty; change this to follow the same guard used for
`avg_merge`/`median_merge`: only compute `fastest = min(...)` and `fastest_h`
when `merge_times` is truthy, otherwise set `fastest_h` to "N/A" (use the same
fallback formatting as `avg_merge`/`median_merge`); reference the
variables/expressions `merge_times`, `avg_merge`, `median_merge`, `fastest`, and
`fastest_h` to locate and update the logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: f146ce00-9082-4ab3-8049-66598f598fed
📒 Files selected for processing (2)
.claude/commands/pr-report.mdcontrib/repo_metrics/weekly_pr_report.py
| merge_times = [p['readyToMergeHours'] for p in self.prs if p.get('readyToMergeHours')] | ||
| avg_merge = f"{sum(merge_times)/len(merge_times):.1f}" if merge_times else "N/A" | ||
| median_merge = f"{sorted(merge_times)[len(merge_times)//2]:.1f}" if merge_times else "N/A" | ||
| fastest = min(self.prs, key=lambda p: p.get('readyToMergeHours') or float('inf')) | ||
| fastest_h = f"{fastest.get('readyToMergeHours', 0):.1f}" | ||
|
|
There was a problem hiding this comment.
Guard against empty PR list to prevent ValueError.
If self.prs is empty, min(self.prs, ...) at line 1220 will raise ValueError: min() arg is an empty sequence. Line 1222 then uses fastest without checking. The existing markdown generator (lines 1143-1147) correctly wraps similar min/max calls inside an if merge_times: check.
🛡️ Proposed fix
merge_times = [p['readyToMergeHours'] for p in self.prs if p.get('readyToMergeHours')]
avg_merge = f"{sum(merge_times)/len(merge_times):.1f}" if merge_times else "N/A"
median_merge = f"{sorted(merge_times)[len(merge_times)//2]:.1f}" if merge_times else "N/A"
-fastest = min(self.prs, key=lambda p: p.get('readyToMergeHours') or float('inf'))
-fastest_h = f"{fastest.get('readyToMergeHours', 0):.1f}"
+if merge_times:
+ fastest = min(self.prs, key=lambda p: p.get('readyToMergeHours') or float('inf'))
+ fastest_h = f"{fastest.get('readyToMergeHours', 0):.1f}"
+else:
+ fastest_h = "N/A"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| merge_times = [p['readyToMergeHours'] for p in self.prs if p.get('readyToMergeHours')] | |
| avg_merge = f"{sum(merge_times)/len(merge_times):.1f}" if merge_times else "N/A" | |
| median_merge = f"{sorted(merge_times)[len(merge_times)//2]:.1f}" if merge_times else "N/A" | |
| fastest = min(self.prs, key=lambda p: p.get('readyToMergeHours') or float('inf')) | |
| fastest_h = f"{fastest.get('readyToMergeHours', 0):.1f}" | |
| merge_times = [p['readyToMergeHours'] for p in self.prs if p.get('readyToMergeHours')] | |
| avg_merge = f"{sum(merge_times)/len(merge_times):.1f}" if merge_times else "N/A" | |
| median_merge = f"{sorted(merge_times)[len(merge_times)//2]:.1f}" if merge_times else "N/A" | |
| if merge_times: | |
| fastest = min(self.prs, key=lambda p: p.get('readyToMergeHours') or float('inf')) | |
| fastest_h = f"{fastest.get('readyToMergeHours', 0):.1f}" | |
| else: | |
| fastest_h = "N/A" |
🤖 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 `@contrib/repo_metrics/weekly_pr_report.py` around lines 1217 - 1222, The code
calls min(self.prs, ...) to compute `fastest` and then formats `fastest_h`,
which raises ValueError when `self.prs` (and thus `merge_times`) is empty;
change this to follow the same guard used for `avg_merge`/`median_merge`: only
compute `fastest = min(...)` and `fastest_h` when `merge_times` is truthy,
otherwise set `fastest_h` to "N/A" (use the same fallback formatting as
`avg_merge`/`median_merge`); reference the variables/expressions `merge_times`,
`avg_merge`, `median_merge`, `fastest`, and `fastest_h` to locate and update the
logic.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8558 +/- ##
=======================================
Coverage 40.34% 40.34%
=======================================
Files 755 755
Lines 93167 93167
=======================================
Hits 37587 37587
Misses 52877 52877
Partials 2703 2703
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Summary
generate_html_report()method toweekly_pr_report.pythat produces a self-contained dark-themed HTML dashboard alongside existing markdown/JSON outputs/pr-reportskill injects Claude-generated highlights into the HTML placeholder after writing the impact analysis reportTest plan
python3 contrib/repo_metrics/weekly_pr_report.py --output-dir /tmp/testand verifyweekly_pr_report.htmlis generated/pr-reportend-to-end and verify highlights are injected into the HTMLdiffthe HTML outputs (excluding timestamp) to confirm determinism🤖 Generated with Claude Code
Summary by CodeRabbit
New Features