feat: add HTML/Markdown reporting and failure diagnostics to verifier (#87)#94
feat: add HTML/Markdown reporting and failure diagnostics to verifier (#87)#94AbiramiR-27 wants to merge 3 commits into
Conversation
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds configurable verification report formatting, diagnostics collection, and report file output for the verify flow. It updates the CLI, report rendering, and tests to cover text, Markdown, and HTML output. ChangesVerification Report Formatting
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/verifier.py`:
- Around line 634-673: print_report currently falls through silently when
format_type is not text, markdown, or html, so add an explicit else branch that
raises ValueError for unknown formats. Keep the fix inside print_report and make
sure the error is raised after the existing text/markdown/html handling so
direct calls and tests fail fast instead of producing no output.
- Around line 342-364: The Markdown table in generate_markdown_report can break
when any cell value contains a pipe character, especially the Details field from
r.detail. Update the table row generation in generate_markdown_report to escape
or sanitize pipes in all cell values before appending them, including detail_val
and any other dynamically inserted fields like expected_val, actual_val, and
r.name, so the report table stays structurally valid.
- Around line 367-627: The HTML report builder in generate_html_report is
inserting unescaped user-controlled values, creating an XSS risk in shared
reports. Add the html module import and escape every interpolated field before
rendering, including ref, each CheckResult field used in rows (name, expected,
actual, detail), and diagnostic keys/values in the diag section. Keep the
formatting logic in generate_html_report unchanged aside from applying escaping
at the insertion points.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab286735-ba72-4b7c-96a3-4a429bcfe7fe
📒 Files selected for processing (3)
src/ovllm.pysrc/verifier.pytests/test_verifier.py
… and format validation
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_verifier.py (1)
224-284: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRipper test coverage for the report formatting — XSS and pipe escaping checks are spot on.
The test method name
test_text_markdown_html_report_generationmentions "text" but doesn't directly assert text-format output. The text path is exercised indirectly through the invalid-format test (which builds the text string before raising), but an explicit assertion on text output would make the name fully accurate. Not blocking — just a heads-up for future polish.🤖 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 `@tests/test_verifier.py` around lines 224 - 284, The report-formatting test name suggests text output, but `test_text_markdown_html_report_generation` never explicitly verifies the text formatter. Add a direct text-format assertion in `ReportFormattingTests` by calling `print_report` with `format_type="text"` and checking the generated output contains the expected report header/content, using the existing `print_report` helper to keep coverage aligned with the test name.
🤖 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.
Nitpick comments:
In `@tests/test_verifier.py`:
- Around line 224-284: The report-formatting test name suggests text output, but
`test_text_markdown_html_report_generation` never explicitly verifies the text
formatter. Add a direct text-format assertion in `ReportFormattingTests` by
calling `print_report` with `format_type="text"` and checking the generated
output contains the expected report header/content, using the existing
`print_report` helper to keep coverage aligned with the test name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 508d5714-a53a-47a5-94c0-914161ea85e2
📒 Files selected for processing (2)
src/verifier.pytests/test_verifier.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/verifier.py
|
Hi @Archit381 The implementation is complete, all tests have been updated, and CodeRabbit's review feedback has been fully addressed. All CI checks are passing. Waiting for your review and merge! |
1 participant
feat: add HTML/Markdown reporting and failure diagnostics to verifier (#87)
Addressed Issues:
Fixes #87
Screenshots/Recordings:
A Markdown report (report.md) and an HTML report (report.html) are generated under the output directory when run with the formatting flags:
Markdown Output Example:
a952...a952...HTML Output Preview:
Generates a dark-themed single-page report with responsive layout grids, check status badges (PASS/FAIL/SKIP), and expected vs actual code block comparisons.
Additional Notes:
This PR implements format selection (
text,markdown, andhtml) and file output capabilities forovllm verify.Core changes include:
--format(choices:text,markdown,html) and--output <filepath>arguments to the verifier CLI insrc/ovllm.py.get_system_diagnostics()insrc/verifier.pyto retrieve active operating system details, Python/PyTorch versions, and hardware accelerators (CUDA/XPU/CPU).src/verifier.py.test_sigstore_verify_ignores_signature_fileintests/test_verifier.pyby mocking themodel_signingdependency search so tests remain independent of local environment packages.ReportFormattingTeststesting custom markdown/html files and CLI integration.AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: Agentic Coding IDE Assistant
Checklist
Summary by CodeRabbit