feat(risk-assessment): add PDF report generation endpoint#21
Merged
Conversation
Reviewer's GuideAdds a new risk assessment PDF report generation capability, including backend data aggregation, a PDF rendering module using printpdf, and an authenticated GET endpoint to download the report, plus associated dependency updates. Sequence diagram for risk assessment PDF report generation endpointsequenceDiagram
actor Client
participant API as RiskAssessmentAPI
participant Service as RiskAssessmentService
participant DB as Database
participant RG as ReportGenerator
Client->>API: GET /v2/risk-assessment/{id}/report
API->>Service: get_report_data(id, db_tx)
Service->>DB: find RiskAssessment by id
DB-->>Service: RiskAssessment or None
alt assessment_not_found
Service-->>API: Ok(None)
API-->>Client: 404 NotFound
else assessment_found
loop documents
Service->>DB: find RiskAssessmentDocuments
DB-->>Service: documents
Service->>DB: find RiskAssessmentCriteria by document
DB-->>Service: criteria
Service->>Service: build scoring_categories
Service->>Service: build report_categories
end
Service->>Service: compute_scoring_result(scoring_categories)
Service-->>API: Some(ReportData)
API->>RG: generate_report(ReportData)
RG-->>API: pdf_bytes
API-->>Client: 200 OK
Note right of API: Content-Type: application/pdf\nContent-Disposition: attachment
end
Class diagram for new risk assessment PDF report generation typesclassDiagram
class RiskAssessmentService {
+get_report_data(assessment_id: &str, db: &impl ConnectionTrait) Result~Option~ReportData~~
}
class ReportData {
+assessment_id: String
+group_id: String
+status: String
+created_at: String
+categories: Vec~ReportCategory~
+scoring: Option~ScoringResult~
}
class ReportCategory {
+category: String
+criteria: Vec~ReportCriterion~
}
class ReportCriterion {
+criterion: String
+completeness: String
+risk_level: String
+score: f64
+what_documented: Vec~String~
+gaps: Vec~String~
+impact_description: Option~String~
+recommendations: Vec~ReportRecommendation~
+details: Option~Value~
}
class ReportRecommendation {
+action: String
+priority: String
}
class ReportWriter {
-doc: PdfDocumentReference
-font: IndirectFontRef
-font_bold: IndirectFontRef
-current_page: PdfPageIndex
-current_layer: PdfLayerIndex
-y: f32
+new(title: &str) Result~ReportWriter~
+layer() PdfLayerReference
+advance(mm: f32) void
+new_page() void
+ensure_space(needed: f32) void
+write_text(text: &str, size: f32, x: f32, font: &IndirectFontRef) void
+title(text: &str) void
+heading(text: &str) void
+subheading(text: &str) void
+bold_line(text: &str) void
+body(text: &str) void
+body_at(text: &str, x: f32) void
+bold_at(text: &str, x: f32) void
+key_value(key: &str, value: &str) void
+bullet(text: &str) void
+paragraph(text: &str) void
+small_text(text: &str) void
+horizontal_rule() void
+spacing(mm: f32) void
+table_row(cells: &[&str], col_widths: &[f32], bold: bool) void
+finish() Result~Vec~u8~~
}
class scoring {
<<module>>
+compute_scoring_result(categories: &Vec~CategoryResult~) ScoringResult
}
class report_generator_module {
<<module>>
+generate_report(data: &ReportData) Result~Vec~u8~~
}
RiskAssessmentService --> ReportData : builds
RiskAssessmentService ..> report_generator_module : calls_generate_report
ReportData *-- ReportCategory
ReportCategory *-- ReportCriterion
ReportCriterion *-- ReportRecommendation
report_generator_module ..> ReportWriter : uses
report_generator_module ..> scoring : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
get_report_data, each document triggers its ownrisk_assessment_criteriaquery, which can lead to N+1 queries for large assessments; consider fetching criteria for all documents in a single query and grouping in memory to reduce DB round-trips. - In
ReportWriter::table_row, truncation is based oncell.len()and slicing&cell[..], which can split multi‑byte UTF‑8 characters; usingcell.chars().take(max_chars)(or a similar character- or grapheme-aware approach) would avoid invalid UTF‑8 in the generated PDF. - The font references in
ReportWriterdo not need to be cloned each time they are used (e.g.&self.font_bold.clone()); passing&self.font_bold/&self.fontdirectly would simplify the code and avoid unnecessary cloning.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `get_report_data`, each document triggers its own `risk_assessment_criteria` query, which can lead to N+1 queries for large assessments; consider fetching criteria for all documents in a single query and grouping in memory to reduce DB round-trips.
- In `ReportWriter::table_row`, truncation is based on `cell.len()` and slicing `&cell[..]`, which can split multi‑byte UTF‑8 characters; using `cell.chars().take(max_chars)` (or a similar character- or grapheme-aware approach) would avoid invalid UTF‑8 in the generated PDF.
- The font references in `ReportWriter` do not need to be cloned each time they are used (e.g. `&self.font_bold.clone()`); passing `&self.font_bold` / `&self.font` directly would simplify the code and avoid unnecessary cloning.
## Individual Comments
### Comment 1
<location path="modules/fundamental/src/risk_assessment/endpoints/mod.rs" line_range="336-337" />
<code_context>
+ return Ok(HttpResponse::NotFound().finish());
+ };
+
+ let pdf_bytes = report_generator::generate_report(&report_data)
+ .map_err(|e| Error::Internal(format!("Failed to generate PDF report: {e}")))?;
+
+ Ok(HttpResponse::Ok()
</code_context>
<issue_to_address>
**🚨 issue (security):** Avoid returning internal error details in the HTTP error message
The current mapping exposes the `generate_report` error text to clients, which can leak implementation details or sensitive data. Instead, log the detailed error on the server and return a generic client message like `"Failed to generate PDF report"` to avoid information disclosure.
</issue_to_address>
### Comment 2
<location path="modules/fundamental/src/risk_assessment/service/report_generator.rs" line_range="144-153" />
<code_context>
+ self.advance(LINE_HEIGHT_TITLE + 4.0);
+ }
+
+ fn heading(&mut self, text: &str) {
+ self.ensure_space(LINE_HEIGHT_HEADING + 3.0);
+ self.advance(3.0);
+ self.write_text(
+ text,
+ FONT_SIZE_HEADING,
+ MARGIN_LEFT,
+ &self.font_bold.clone(),
+ );
+ self.advance(LINE_HEIGHT_HEADING + 2.0);
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** `heading` uses `ensure_space` without accounting for the extra pre-advance spacing
`heading` currently reserves `LINE_HEIGHT_HEADING + 3.0`, but then advances `3.0 + LINE_HEIGHT_HEADING + 2.0` in total. This can let the cursor run past the bottom margin when a heading is near the page end. `ensure_space` should be called with the full planned vertical consumption (e.g. `3.0 + LINE_HEIGHT_HEADING + 2.0`), or the function refactored so `ensure_space` always matches the total advances in the body.
</issue_to_address>
### Comment 3
<location path="modules/fundamental/src/risk_assessment/service/report_generator.rs" line_range="138-140" />
<code_context>
+ .use_text(text, size, Mm(x), Mm(self.y), font);
+ }
+
+ fn title(&mut self, text: &str) {
+ self.ensure_space(LINE_HEIGHT_TITLE + 4.0);
+ self.write_text(text, FONT_SIZE_TITLE, MARGIN_LEFT, &self.font_bold.clone());
+ self.advance(LINE_HEIGHT_TITLE + 4.0);
+ }
</code_context>
<issue_to_address>
**suggestion:** Avoid cloning font references when writing text
Across helpers like `title`, `heading`, `subheading`, `bold_line`, and `body`, you’re passing `&self.font.clone()` / `&self.font_bold.clone()` into `write_text`. Since `IndirectFontRef` is already stored on `self`, you can just pass `&self.font` or `&self.font_bold` directly, avoiding the temporary clone and simplifying the call.
Suggested implementation:
```rust
fn write_text(&self, text: &str, size: f32, x: f32, font: &IndirectFontRef) {
self.layer()
.use_text(text, size, Mm(x), Mm(self.y), font);
}
```
```rust
fn title(&mut self, text: &str) {
self.ensure_space(LINE_HEIGHT_TITLE + 4.0);
self.write_text(text, FONT_SIZE_TITLE, MARGIN_LEFT, &self.font_bold);
self.advance(LINE_HEIGHT_TITLE + 4.0);
}
```
Based on your comment, similar changes are likely needed in the other helpers:
- `heading`, `subheading`, `bold_line`, and `body` should pass `&self.font_bold` / `&self.font` (and any other stored `IndirectFontRef`s) directly to `write_text` instead of `&self.font_bold.clone()` / `&self.font.clone()`.
- Search this file for `.font.clone()` and `.font_bold.clone()` and replace those occurrences in calls to `write_text` with `&self.font` / `&self.font_bold` respectively.
No changes are needed to the `write_text` signature; it already takes a `&IndirectFontRef`, which works directly with the stored fields.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+138
to
+140
| fn title(&mut self, text: &str) { | ||
| self.ensure_space(LINE_HEIGHT_TITLE + 4.0); | ||
| self.write_text(text, FONT_SIZE_TITLE, MARGIN_LEFT, &self.font_bold.clone()); |
There was a problem hiding this comment.
suggestion: Avoid cloning font references when writing text
Across helpers like title, heading, subheading, bold_line, and body, you’re passing &self.font.clone() / &self.font_bold.clone() into write_text. Since IndirectFontRef is already stored on self, you can just pass &self.font or &self.font_bold directly, avoiding the temporary clone and simplifying the call.
Suggested implementation:
fn write_text(&self, text: &str, size: f32, x: f32, font: &IndirectFontRef) {
self.layer()
.use_text(text, size, Mm(x), Mm(self.y), font);
} fn title(&mut self, text: &str) {
self.ensure_space(LINE_HEIGHT_TITLE + 4.0);
self.write_text(text, FONT_SIZE_TITLE, MARGIN_LEFT, &self.font_bold);
self.advance(LINE_HEIGHT_TITLE + 4.0);
}Based on your comment, similar changes are likely needed in the other helpers:
heading,subheading,bold_line, andbodyshould pass&self.font_bold/&self.font(and any other storedIndirectFontRefs) directly towrite_textinstead of&self.font_bold.clone()/&self.font.clone().- Search this file for
.font.clone()and.font_bold.clone()and replace those occurrences in calls towrite_textwith&self.font/&self.font_boldrespectively.
No changes are needed to thewrite_textsignature; it already takes a&IndirectFontRef, which works directly with the stored fields.
Owner
Author
There was a problem hiding this comment.
[sdlc-workflow/verify-pr] Classified as suggestion — removing unnecessary .clone() on IndirectFontRef is a minor optimization. Not documented in CONVENTIONS.md and no established codebase pattern (only printpdf usage). No sub-task created.
Owner
Author
Verification Report for JIRAPLAY-1423 (commit f0d3b9c)
Sub-tasks Created
Overall: FAILTwo issues require attention before merge:
This comment was AI-generated by sdlc-workflow/verify-pr v0.5.10. |
Owner
Author
Verification Report for JIRAPLAY-1423 (commit da68556)
Sub-task Created (CI failure)
Overall: FAILOne CI issue remains: clippy This comment was AI-generated by sdlc-workflow/verify-pr v0.5.10. |
Add GET /v2/risk-assessment/{id}/report endpoint that generates a
formatted PDF report from assessment results stored in the database.
The report includes:
- Executive summary with overall risk score and category breakdown
- Detailed criteria assessments with enriched fields (what_documented,
gaps, impact, recommendations)
- Risk assessment matrix with threat scenarios
Uses printpdf with built-in Helvetica fonts for zero-dependency PDF
generation.
Implements JIRAPLAY-1423
Assisted-by: Claude Code
… endpoint Log the detailed error server-side and return a generic message to HTTP clients instead of including the internal error text in the response, preventing information disclosure. Implements JIRAPLAY-1427 Assisted-by: Claude Code
…go fmt Fix heading() to reserve the correct vertical space (3.0 + 7.0 + 2.0 = 12.0mm) via ensure_space to prevent cursor overflow past the bottom margin. Apply cargo fmt to fix all formatting diffs. Implements JIRAPLAY-1428 Assisted-by: Claude Code
Collapse nested if-let for threat_scenarios and as_array() into a single if-let chain to satisfy clippy::collapsible_if. Implements JIRAPLAY-1429 Assisted-by: Claude Code
Owner
Author
Verification Report for JIRAPLAY-1423 (commit 207f50a)
Sub-task Created (CI failure)
Overall: FAILCI 'Export and Validate Generated Openapi Spec' step fails — This comment was AI-generated by sdlc-workflow/verify-pr v0.5.10. |
Run cargo xtask openapi to include the new generateRiskAssessmentReport endpoint in the spec. Implements JIRAPLAY-1430 Assisted-by: Claude Code
Owner
Author
Verification Report for JIRAPLAY-1423 (commit e883c61)
Overall: WARNAll functional checks pass. The only WARN is scope containment due to expected dependency and generated spec files. PR is ready for human review. This comment was AI-generated by sdlc-workflow/verify-pr v0.5.10. |
…completeness score Redesign the PDF report to match the SAR Completeness Report Viewer: - Section 1: Overall Rating with completeness-based score - Section 2: Criteria Summary Table - Section 3: Risk Assessments with likelihood/impact/threat scenarios - Section 4: Risk Prioritization with critical gaps and top risks - Section 5: Criteria Assessments with documented/gaps/recommendations Replace weighted NIST score with completeness formula: (complete * 1.0 + partial * 0.5) / total Add risk_prioritization JSONB column to risk_assessment_document via migration. Store LLM risk_prioritization during document processing. Implements JIRAPLAY-1432 Assisted-by: Claude Code
… into single write Remove store_risk_prioritization() and merge its logic into the existing "mark as processed" update in process_document(). This eliminates two sequential UPDATEs on the same row and ensures risk_prioritization is always written (including None on re-runs), preventing stale data from persisting. Implements JIRAPLAY-1433 Assisted-by: Claude Code
Owner
Author
Verification Report for JIRAPLAY-1423 (commit 4e63ee7)
Overall: WARNAll CI checks pass. All review feedback addressed. PR ready for human review. This comment was AI-generated by sdlc-workflow/verify-pr v0.5.11. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GET /v2/risk-assessment/{id}/reportendpoint that generates a formatted PDF report from assessment resultsreport_generator.rsmodule usingprintpdfwith built-in Helvetica fonts (no external font dependencies)get_report_data()service method fetching full assessment data including enriched fields (what_documented, gaps, impact, recommendations) not exposed in the API responseImplements JIRAPLAY-1423
Test plan
cargo check -p trustify-module-fundamentalcompiles without errors or warningscargo buildsucceedsGET /v2/risk-assessment/{id}/reporton an assessment with processed documents and verify PDF contains all sectionsContent-Type: application/pdfandContent-Dispositionheaders🤖 Generated with Claude Code
Summary by Sourcery
Add a new PDF report generation capability for risk assessments and expose it via an authenticated HTTP endpoint.
New Features:
Enhancements:
Build: