Conversation
…verflow Large operations with many detection strategies caused LayoutError because KeepTogether forced header+table onto a single page. KeepTogetherSplitAtTop allows the table to split across pages when it exceeds the frame height. This matches the pattern already used by all other debrief report sections. Also fixes header/footer rendering on landscape pages by using actual page dimensions (canvas._pagesize) instead of doc.width/doc.height which retain portrait values regardless of the active PageTemplate orientation.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses ReportLab LayoutError crashes when generating the “TTPs and V18 Detections” section for operations that produce very large detection tables, particularly in landscape mode.
Changes:
- Switch the detections appendix wrapper from
KeepTogethertoKeepTogetherSplitAtTopso large tables can split across pages while keeping the header with the first fragment. - Fix header/footer positioning logic to use the actual canvas page size so landscape pages render correctly.
- Add a regression test that builds a PDF from a large detections section and asserts successful PDF generation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
app/debrief-sections/ttps_detections.py |
Replaces KeepTogether with KeepTogetherSplitAtTop around header+table blocks to avoid LayoutError on tall tables. |
app/objects/c_story.py |
Uses canvas._pagesize to compute usable dimensions so header/footer placement works correctly for landscape pages. |
tests/test_pdf_large_table.py |
Adds an integration-style regression test that generates flowables for a large operation and builds an actual landscape PDF. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/test_pdf_large_table.py
Outdated
Comment on lines
+14
to
+19
| from unittest.mock import MagicMock, patch | ||
|
|
||
| from reportlab.lib.pagesizes import letter, landscape as to_landscape | ||
| from reportlab.lib.styles import getSampleStyleSheet | ||
| from reportlab.lib.units import inch | ||
| from reportlab.platypus import SimpleDocTemplate, PageBreak, Frame, PageTemplate |
Contributor
Author
There was a problem hiding this comment.
Fixed — removed MagicMock, patch, inch, PageBreak, Frame, and PageTemplate imports.
tests/test_pdf_large_table.py
Outdated
Comment on lines
+95
to
+96
| paw_to_platform = {agent.paw: agent.platform} | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Fixed — removed unused variable.
Comment on lines
+70
to
+77
| # Use technique IDs that exist in the ATT&CK v18 mapping so detection rows are generated. | ||
| # Even if some TIDs have no detections, the ones that do will produce many rows. | ||
| technique_ids = [ | ||
| 'T1083', 'T1547.001', 'T1560.001', 'T1548.001', | ||
| 'T1059.001', 'T1053.005', 'T1021.002', 'T1021.001', | ||
| 'T1055.001', 'T1003.001', 'T1070.004', 'T1071.001', | ||
| 'T1036.005', 'T1105', | ||
| ] |
Contributor
Author
There was a problem hiding this comment.
Fixed — added assertion that at least one Table flowable is generated, guarding against ATT&CK data changes making the test vacuous.
…op_id bug - Remove unused imports (MagicMock, patch, inch, PageBreak, Frame, PageTemplate) and unused variable (paw_to_platform) from test_pdf_large_table.py - Fix file handle leak in c_story.py: use context manager for svg.write() - Add null check for missing viewBox attribute in adjust_icon_svgs() - Fix stale op_id variable in build_steps_d3() — use operation.id instead
Row-spans on the AN/Platform/Statement columns prevented ReportLab from splitting large detection tables across pages, causing LayoutError even with KeepTogetherSplitAtTop. Tables with many analytics rows (e.g., SMB Lateral Movement with 14+ techniques) would exceed the 564pt landscape frame height and crash. The fix: - Separate the header block from the table (header kept together, table flows freely) - Disable auto-generated row-spans that blocked page splitting - Enable splitByRow=True and splitInRow=True for robust splitting
…overlap - Tactics column: wrap tactic names in Paragraph with wordWrap='CJK' and widen column from 0.75in to 1.10in so names like "Privilege-escalation" don't overflow into adjacent columns - Detection header/table gap: re-combine header block and table into a single KeepTogetherSplitAtTop flowable (the gap was caused by Story.append adding 12pt spacer between separate flowables) - Landscape header overlap: use actual frame margins for header/footer positioning instead of doc-level portrait margins. Landscape frames use 18pt margins but doc.topMargin was 84pt, causing the "OPERATIONS DEBRIEF" title to draw over the table content area
- Tactics column: use .title() for proper capitalization of each word (e.g., "Lateral-Movement" not "Lateral-movement") and insert <br/> after hyphens so multi-word tactics display on separate lines - Landscape header: on continuation pages, detect landscape orientation and render a compact 8pt header with thin line at page top instead of the full 18pt header that was overlapping the table content area (landscape frames use 18pt margins vs portrait's 84pt)
The 18pt landscape margins are too narrow to render any header text without overlapping the detection table content. Landscape continuation pages now render only the page number footer.
deacon-mp
changed the title
Adds assertion that at least one Table flowable is generated, guarding against ATT&CK data changes that could reduce the table below the split threshold and make the regression test vacuous.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a regression fix and tests to prevent ReportLab LayoutError when rendering large “TTPs / Detections” tables, particularly in landscape PDFs, by allowing tables to split across pages while keeping headers attached.
Changes:
- Replace
KeepTogetherwithKeepTogetherSplitAtTopfor detection header+table flowables so oversized tables can split across pages. - Adjust PDF header/footer rendering to use the actual page size (important for landscape templates).
- Add a regression test that builds a real PDF from a large detections table and asserts it does not crash.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/test_pdf_large_table.py | Adds regression coverage for large detection tables and asserts the new flowable wrapper is used. |
| app/objects/c_story.py | Uses the canvas page size when positioning header/footer elements so landscape pages render correctly. |
| app/debrief-sections/ttps_detections.py | Switches to KeepTogetherSplitAtTop to allow large tables to split across pages without LayoutError. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+88
to
+101
| @pytest.mark.asyncio | ||
| async def test_large_operation_does_not_crash(self, large_operation): | ||
| """Build a landscape PDF with a large detection table and verify no exception.""" | ||
| op, agent = large_operation | ||
| section = TTP_DET_MODULE.DebriefReportSection() | ||
| styles = getSampleStyleSheet() | ||
|
|
||
| paw_to_platform = {agent.paw: agent.platform} | ||
|
|
||
| flowables = await section.generate_section_elements( | ||
| styles, | ||
| operations=[op], | ||
| agents=[agent], | ||
| ) |
tests/test_pdf_large_table.py
Outdated
Comment on lines
+14
to
+19
| from unittest.mock import MagicMock, patch | ||
|
|
||
| from reportlab.lib.pagesizes import letter, landscape as to_landscape | ||
| from reportlab.lib.styles import getSampleStyleSheet | ||
| from reportlab.lib.units import inch | ||
| from reportlab.platypus import SimpleDocTemplate, PageBreak, Frame, PageTemplate |
tests/test_pdf_large_table.py
Outdated
Comment on lines
+95
to
+96
| paw_to_platform = {agent.paw: agent.platform} | ||
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds safeguards and regression coverage to prevent ReportLab PDF generation failures when detections tables become too large to fit on a single landscape page, while also improving debrief PDF header/footer placement on landscape pages.
Changes:
- Switch detections appendix rendering from
KeepTogethertoKeepTogetherSplitAtTopand enable table splitting to avoidLayoutError. - Update PDF header/footer rendering to account for landscape frame margins (18pt) instead of portrait doc margins.
- Add a regression test that builds a PDF for a “large” detections section; adjust TTP table tactic column formatting/widths; fix D3 graph operation ID wiring.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_pdf_large_table.py |
Adds regression tests intended to ensure large detections tables don’t crash PDF generation. |
app/objects/c_story.py |
Computes effective margins per page orientation/frame and updates header/footer drawing accordingly. |
app/debrief-sections/ttps_detections.py |
Enables multi-page splitting for large detections tables and keeps header attached to the first fragment. |
app/debrief-sections/tactic_technique_table.py |
Improves tactic cell formatting (multi-line hyphen splits) and adjusts column widths. |
app/debrief_svc.py |
Fixes D3 graph output to consistently use operation.id when creating nodes/links. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/test_pdf_large_table.py
Outdated
Comment on lines
+102
to
+117
| # Verify the table is actually large enough to require page splitting. | ||
| # A landscape frame is ~564pt; the table must exceed this to exercise | ||
| # the split path. If ATT&CK data changes reduce the row count, this | ||
| # assertion will catch it before the regression test becomes vacuous. | ||
| from reportlab.platypus import Table | ||
| tables = [f for f in flowables if isinstance(f, Table)] | ||
| assert len(tables) > 0, "Expected at least one Table flowable" | ||
|
|
||
| # Build an actual PDF in a landscape frame matching debrief_gui.py | ||
| buf = io.BytesIO() | ||
| lw, lh = to_landscape(letter) | ||
| margin = 18 | ||
| doc = SimpleDocTemplate(buf, pagesize=(lw, lh), | ||
| leftMargin=margin, rightMargin=margin, | ||
| topMargin=margin, bottomMargin=margin) | ||
|
|
Comment on lines
+497
to
+503
| valign_cmds.append(('VALIGN', (col_idx, r_start), (col_idx, r), 'MIDDLE')) | ||
| r += 1 | ||
|
|
||
| # Spanning for AN / Platform / Statement | ||
| _add_spans_for_column(0) | ||
| _add_spans_for_column(1) | ||
| _add_spans_for_column(2) | ||
| # NOTE: Row-spanning for AN/Platform/Statement columns is intentionally | ||
| # disabled. Row spans prevent ReportLab from splitting the table across | ||
| # pages, causing LayoutError for detection tables with many rows. | ||
| # The table uses repeatRows=2 and splitByRow=True to allow page breaks. |
- Filter detection tables by 8-column width to avoid matching the section-band header table in the assertion - Remove dead row-spanning code and outdated docstring from _build_det_table — spans are disabled to allow page splitting
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a ReportLab LayoutError encountered when rendering very large Detection Strategy tables in the debrief PDF (landscape pages), and adds a regression test to ensure large tables can be rendered without crashing.
Changes:
- Update detections appendix rendering to use
KeepTogetherSplitAtTopand enable table splitting so large tables can span pages. - Improve header/footer margin calculations for landscape pages by deriving margins from the active frame.
- Add a regression test that builds a landscape PDF containing a detections table.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_pdf_large_table.py | Adds an async regression test that generates detections flowables and builds a landscape PDF to ensure no LayoutError. |
| app/objects/c_story.py | Adjusts header/footer rendering to compute margins correctly on landscape pages; improves SVG handling robustness. |
| app/debrief-sections/ttps_detections.py | Switches from KeepTogether to KeepTogetherSplitAtTop and removes row-spans to allow large tables to split across pages. |
| app/debrief-sections/tactic_technique_table.py | Improves tactic cell layout by using a centered Paragraph and splitting hyphenated tactic names across lines; tweaks column widths. |
| app/debrief_svc.py | Fixes build_steps_d3() to reference the correct operation id when building nodes/links. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+110
to
+113
| # Build an actual PDF in a landscape frame matching debrief_gui.py | ||
| buf = io.BytesIO() | ||
| lw, lh = to_landscape(letter) | ||
| margin = 18 |
Ensures the test actually exercises the page-split code path by asserting the tallest detection table exceeds the available landscape frame height.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a ReportLab PDF generation failure when debrief “detections” tables become too large, by enabling page-splitting behavior and adjusting related layout behaviors in the debrief report/GUI.
Changes:
- Update the detections appendix to use
KeepTogetherSplitAtTopand configure tables to split across pages (avoidingLayoutErroron large tables). - Improve PDF header/footer rendering for landscape pages by basing placement on the active frame margins.
- Add a regression test for large detections tables; adjust tactic/technique table tactic rendering; fix operation ID usage in the “steps” D3 graph output.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_pdf_large_table.py | Adds a regression test intended to reproduce the large-table LayoutError and assert the new split-friendly behavior. |
| app/objects/c_story.py | Updates header/footer drawing to respect landscape frame margins and makes SVG adjustment more robust. |
| app/debrief-sections/ttps_detections.py | Switches to KeepTogetherSplitAtTop, removes row-spans that block splitting, and enables table splitting options. |
| app/debrief-sections/tactic_technique_table.py | Improves tactic cell rendering (centered Paragraph with hyphenated line breaks) and adjusts column widths. |
| app/debrief_svc.py | Fixes incorrect operation IDs in the steps graph payload by using operation.id consistently. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/test_pdf_large_table.py
Outdated
Comment on lines
+106
to
+107
| det_tables = [f for f in flowables | ||
| if isinstance(f, Table) and len(f._argW) == 8] |
tests/test_pdf_large_table.py
Outdated
Comment on lines
+106
to
+107
| det_tables = [f for f in flowables | ||
| if isinstance(f, Table) and len(f._argW) == 8] |
Comment on lines
+54
to
62
| page_w, page_h = canvas._pagesize | ||
| lm, rm, tm, bm = Story._page_margins(canvas, doc) | ||
|
|
||
| # Header | ||
| caldera_logo = "./plugins/debrief/static/img/caldera.png" | ||
| im = Image(caldera_logo, 1.5 * inch, 1 * inch) | ||
| im.drawOn(canvas, doc.leftMargin, doc.height + doc.topMargin - im.drawHeight / 2) | ||
| header_y = page_h - tm | ||
| im.drawOn(canvas, lm, header_y - im.drawHeight / 2) | ||
|
|
- Unwrap detection tables from KeepTogetherSplitAtTop before asserting height, use row count instead of _argW for identification - Skip header logo on landscape first pages where 18pt margin is too narrow for the 1-inch logo
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request addresses a ReportLab LayoutError regression when generating PDFs containing large detection tables by allowing tables to split across pages, and adds a regression test to ensure large tables can be rendered without crashing.
Changes:
- Update the detections appendix flowables to use
KeepTogetherSplitAtTopand configure the ReportLabTableto split across pages. - Adjust PDF header/footer rendering to account for landscape page/frame margins and harden SVG viewBox handling.
- Add a regression test that builds a real landscape PDF containing a large detections table.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_pdf_large_table.py | Adds a regression test that builds a landscape PDF and asserts large detection tables split without LayoutError. |
| app/objects/c_story.py | Reworks header/footer placement logic (including landscape handling) and improves SVG write/viewBox robustness. |
| app/debrief-sections/ttps_detections.py | Replaces KeepTogether with KeepTogetherSplitAtTop, removes row-spanning, and enables table splitting to prevent LayoutError. |
| app/debrief-sections/tactic_technique_table.py | Improves tactic cell rendering (Paragraph + hyphen line breaks) and adjusts column widths. |
| app/debrief_svc.py | Fixes D3 graph node/link operation IDs to use operation.id rather than a stale loop variable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+61
to
+72
| caldera_logo = "./plugins/debrief/static/img/caldera.png" | ||
| im = Image(caldera_logo, 1.5 * inch, 1 * inch) | ||
| header_y = page_h - tm | ||
| im.drawOn(canvas, lm, header_y - im.drawHeight / 2) | ||
|
|
||
| if Story._header_logo_path: | ||
| Story.draw_header_logo(canvas, doc, Story._header_logo_path) | ||
|
|
||
| canvas.setStrokeColor(colors.maroon) | ||
| canvas.setLineWidth(4) | ||
| canvas.line(doc.leftMargin + im.drawWidth + 5, | ||
| doc.height + doc.topMargin, | ||
| doc.width + doc.leftMargin, | ||
| doc.height + doc.topMargin) | ||
| canvas.setStrokeColor(colors.maroon) | ||
| canvas.setLineWidth(4) | ||
| canvas.line(lm + im.drawWidth + 5, header_y, | ||
| page_w - rm, header_y) |
app/objects/c_story.py
Outdated
Comment on lines
+88
to
+102
| # Header | ||
| if Story._header_logo_path: | ||
| Story.draw_header_logo(canvas, doc, Story._header_logo_path) | ||
|
|
||
| canvas.setFillColor(colors.maroon) | ||
| canvas.setFont('Helvetica-Bold', 18) | ||
| canvas.drawString(doc.leftMargin, doc.height + doc.topMargin * 1.25, 'OPERATIONS DEBRIEF') | ||
| canvas.setStrokeColor(colors.maroon) | ||
| canvas.setLineWidth(4) | ||
| canvas.line(doc.leftMargin, | ||
| doc.height + doc.topMargin * 1.25 - 5, | ||
| doc.width + doc.leftMargin, | ||
| doc.height + doc.topMargin * 1.25 - 5) | ||
| is_landscape = page_w > page_h | ||
| if not is_landscape: | ||
| # Portrait pages have room for the full header | ||
| header_y = page_h - tm * 0.75 | ||
| canvas.setFillColor(colors.maroon) | ||
| canvas.setFont('Helvetica-Bold', 18) | ||
| canvas.drawString(lm, header_y, 'OPERATIONS DEBRIEF') | ||
| canvas.setStrokeColor(colors.maroon) | ||
| canvas.setLineWidth(4) | ||
| canvas.line(lm, header_y - 5, page_w - rm, header_y - 5) | ||
| # Landscape pages: skip header — 18pt margin is too narrow |
…on portrait - header_footer_first: position caldera logo above content frame in top margin area - header_footer_rest: move custom header logo inside portrait check so landscape pages skip it entirely
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a ReportLab LayoutError regression when generating the “Detection Strategies” PDF section with large tables by allowing tables to split across pages in landscape layout, and adds a regression test to cover the scenario.
Changes:
- Replace
KeepTogetherwithKeepTogetherSplitAtTopfor detection header+table blocks and enable table splitting (splitByRow/splitInRow) to avoidLayoutError. - Adjust PDF header/footer rendering to better respect landscape frame margins and harden SVG handling.
- Add a new pytest regression test that builds a landscape PDF containing a large detections table.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/test_pdf_large_table.py |
Adds regression coverage for large detection tables building successfully into a landscape PDF. |
app/objects/c_story.py |
Updates header/footer positioning to be aware of landscape frame margins; improves SVG writing and viewBox handling. |
app/debrief-sections/ttps_detections.py |
Switches to KeepTogetherSplitAtTop and removes row-spanning to allow multi-page table splitting; enables table split flags. |
app/debrief-sections/tactic_technique_table.py |
Tweaks tactic cell formatting (centered paragraph + hyphen line breaks) and adjusts column widths. |
app/debrief_svc.py |
Fixes D3 graph generation to use operation.id instead of a stale op_id variable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+442
to
+448
| def _build_det_table(self, rows): | ||
| '''Build the 8-column detections table with MITRE-style formatting. | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # AUTO-GENERATE ROW SPANS FOR IDENTICAL CELL VALUES | ||
| # ------------------------------------------------------------------ | ||
| start = 2 # row 0–1 = header rows | ||
| end = len(rows) | ||
| valign_cmds = [] # stores ('VALIGN', (col,r1),(col,r2),'MIDDLE') | ||
|
|
||
| def _add_spans_for_column(col_idx): | ||
| ''' | ||
| Detect consecutive identical values in rows[col_idx], | ||
| SPAN them, and emit a VALIGN rule to center vertically. | ||
| ''' | ||
| r = start | ||
| while r < end: | ||
| r_start = r | ||
| cell = rows[r][col_idx] | ||
|
|
||
| # Find the consecutive matching block | ||
| while r + 1 < end and rows[r + 1][col_idx] == cell: | ||
| r += 1 | ||
|
|
||
| if r > r_start: # spanning required | ||
| span_cmds.append(('SPAN', (col_idx, r_start), (col_idx, r))) | ||
| valign_cmds.append(('VALIGN', (col_idx, r_start), (col_idx, r), 'MIDDLE')) | ||
| r += 1 | ||
|
|
||
| # Spanning for AN / Platform / Statement | ||
| _add_spans_for_column(0) | ||
| _add_spans_for_column(1) | ||
| _add_spans_for_column(2) | ||
| Row-spanning for repeated AN/Platform/Statement values is intentionally | ||
| not used — row spans prevent ReportLab from splitting the table across | ||
| pages, causing LayoutError for large detection tables. | ||
| ''' |
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
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
Test plan