Add a section for recommendations or remediation steps. #89
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds recommendations support to AuditReportGenerator: a recommendations list, methods to add single or multiple recommendations, and inclusion of recommendations in generated JSON/Markdown/HTML reports with timestamping via a module-level datetime import. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant ARG as AuditReportGenerator
Client->>ARG: __init__(auditor_name, contract_name)
Note right of ARG: capture audit_timestamp
Client->>ARG: add_static_results(list)
Client->>ARG: add_dynamic_results(list)
Client->>ARG: add_scores(list)
Client->>ARG: set_metadata(dict)
rect rgba(220,235,255,0.35)
Client->>ARG: add_recommendation(str)
Client->>ARG: add_recommendations(list)
Note left of ARG: recommendations appended to internal list
end
Client->>ARG: generate_report()
Note right of ARG: assemble sections including recommendations, summary stats, metadata
ARG-->>Client: report (dict)
alt export_format == "markdown"
Client->>ARG: export_report("markdown")
ARG-->>Client: Markdown string with "Recommendations / Remediation Steps"
else export_format == "html"
Client->>ARG: export_report("html")
ARG-->>Client: HTML string with recommendations list
else export_format == "json"
Client->>ARG: export_report("json")
ARG-->>Client: JSON string including recommendations
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 2
🧹 Nitpick comments (5)
audit_engine/core/report_generator.py (5)
42-42: Move the import to the top of the file.The
import datetimestatement should be moved to the top of the file with the other imports to follow Python PEP 8 conventions and improve code readability.import json +import datetime from typing import List, Dict, AnyAnd remove the misplaced import:
- import datetime
9-9: Fix type annotations to be explicit about Optional parameters.The type checker correctly identifies that parameters with default values of
Noneshould be explicitly annotated asOptional[T].+from typing import List, Dict, Any, Optional - def filter_findings(self, severity: str = None, finding_type: str = None) -> Dict[str, Any]: + def filter_findings(self, severity: Optional[str] = None, finding_type: Optional[str] = None) -> Dict[str, Any]: - def __init__(self, auditor_name: str = None, contract_name: str = None): + def __init__(self, auditor_name: Optional[str] = None, contract_name: Optional[str] = None):Also applies to: 44-44
102-102: Simplify exception message.Consider using a shorter, more concise error message or define a custom exception class for better error handling practices.
- raise ValueError(f"Unsupported output format: {output_format}") + raise ValueError(f"Unsupported format: {output_format}")
79-91: Consider using consistent data structures with existing codebase.The report generator creates a custom report structure, but the codebase already has an
AnalysisResultclass inaudit_engine/core/schemas.pythat includes similar fields likefindings,total_findings,severity_distribution, and metadata. Consider aligning with or extending the existing schema for consistency.You might want to integrate with the existing
AnalysisResultstructure or provide a method to convert fromAnalysisResultto your report format to maintain consistency across the codebase.
53-57: Add input validation for recommendations.The methods don't validate that recommendations are non-empty strings or that the input list contains valid recommendations.
def add_recommendation(self, recommendation: str): + if not recommendation or not isinstance(recommendation, str): + raise ValueError("Recommendation must be a non-empty string") self.recommendations.append(recommendation) def add_recommendations(self, recommendations: list): + if not isinstance(recommendations, list): + raise TypeError("Recommendations must be a list") + for rec in recommendations: + if not isinstance(rec, str) or not rec: + raise ValueError("All recommendations must be non-empty strings") self.recommendations.extend(recommendations)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
audit_engine/core/report_generator.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
audit_engine/core/report_generator.py (4)
audit_engine/core/engine.py (1)
AuditEngine(28-471)audit_engine/core/schemas.py (1)
AnalysisResult(123-179)audit_engine/dynamic_analysis/__init__.py (1)
AnalysisResult(58-71)audit_engine/common/model.py (1)
Finding(9-21)
🪛 Ruff (0.12.2)
audit_engine/core/report_generator.py
9-9: PEP 484 prohibits implicit Optional
Convert to Optional[T]
(RUF013)
9-9: PEP 484 prohibits implicit Optional
Convert to Optional[T]
(RUF013)
44-44: PEP 484 prohibits implicit Optional
Convert to Optional[T]
(RUF013)
44-44: PEP 484 prohibits implicit Optional
Convert to Optional[T]
(RUF013)
102-102: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (1)
audit_engine/core/report_generator.py (1)
8-28: Ensure static_results/dynamic_results are initialized before calling filter_findings.The method reads self.static_results and self.dynamic_results and will raise AttributeError if those attributes aren't set — initialize them in init or add guards/defaults. No call sites for filter_findings were found in the repo.
aleskrin
force-pushed
the
etienne-reporter
branch
from
b801c59 to
ef7b0b4
Compare
Added a section for recommendations or remediation steps.
Summary by CodeRabbit