Repository navigation
fix(report): colour executive summary cards and read document_count - #474
Conversation
Security Scan ResultsASH Security Scan Report
Scan Metadata
SummaryScanner ResultsThe table below shows findings by scanner, with status based on severity thresholds and dependencies:
Top 4 HotspotsFiles with the highest number of security findings:
Detailed FindingsShow 7 actionable findingsFinding 1: SECRET-HEX-HIGH-ENTROPY-STRING
Description: Code Snippet: Finding 2: B310
Description: Code Snippet: Finding 3: B108
Description: Code Snippet: Finding 4: B307
Description: Code Snippet: Finding 5: B307
Description: Code Snippet: Finding 6: B102
Description: Code Snippet: Finding 7: B310
Description: Code Snippet: Report generated by Automated Security Helper (ASH) at 2026-11-11T01:20:17+00:00 |
|
I'm taking this review. I checked the core of it on this head: the six new tests fail on current One thing for you now: since #455 merged, this conflicts with |
|
Thanks @digalaviz! Merged |
vawsgit
left a comment
There was a problem hiding this comment.
✅ LGTM!
Verified at head 7eedb8c and on a local merge with current dev (which includes #549):
- The merge with
devis clean._json_for_scriptfrom #549 and thehtml.escape(title)calls are intact. - Real reports match the default thresholds. A bulk
ProcessEvaluationof 3 documents gives Precision 0.444 orange, Recall 1.000 green, F1 0.615 yellow, and the title "Evaluation Report - 3 Documents". A dict withdocument_count: 7shows 7.document_count=Noneshows 1, wheredevshowed "None Documents". - Escaping holds:
document_count='<script>alert(1)</script>'renders as<script>. Card colours come only from the fixed palette. tests/reportinggives 159 passed at head and 168 on the merge. Mutations are caught: thehasattrguard restored,isinstance(value, float), the dict branch dropped, theNonefallback dropped,section_generatorreverted togetattr.- All "Done when" items in #467 are met. The CHANGELOG entry is under
[Unreleased].
Non-blocking (low):
- No test pins the
_get_document_countchange (html_reporter.py:309) or thenot isinstance(value, bool)check (section_generator.py:47). Reverting either one passes all tests. The behaviour itself is correct, measured by hand. - Integer card values now render as
1.000in a threshold colour, where they used to render as1in#010101. The CHANGELOG does not mention it.
digalaviz
left a comment
There was a problem hiding this comment.
Approved. The fix does what #467 asks, measured at 7eedb8c and on its merge with current dev (c64e6bb), same inputs through EvaluationHTMLReporter().generate_report:
| input | dev |
this PR |
|---|---|---|
ProcessEvaluation, no document_count |
title Evaluation Report - None Documents, card None |
1 Documents, card 1 |
ProcessEvaluation(document_count=3) |
3 |
3 |
dict with document_count: 7 |
card 1 |
card 7 |
precision 0.444, recall 1, F1 0.615 |
all #007bff, recall shown as 1 in #010101 |
#fd7e14, #28a745 (1.000), #ffc107 |
The full suite passes on the merge: 4116 passed, 13 skipped, 1 xfailed. tests/reporting gives 159 passed at the head.
One gap, which we will close in a follow-up PR so this one can merge now: no test pins the bulk title. With _get_document_count (html_reporter.py:309) reverted to its dev body, all 159 reporting tests still pass and the title goes back to None Documents.
Non-blocking, also for the follow-up:
ReportResult.metadata["document_count"]changes too:Nonebecomes1for aProcessEvaluationwithout a count, and a results dict now reports its owndocument_countinstead of1. Correct, but the CHANGELOG entry only names the card.- The
not isinstance(value, bool)check (section_generator.py:47) has no test either.
Fixes #467
hasattr(self.viz_engine, '_get_performance_color')guard so float metric cards useColorUtils.get_performance_color(value, config.color_thresholds).DataExtractor.get_document_countnow reads a results dict'sdocument_countkey and falls back to 1 when the value isNone.SectionGeneratorandHTMLReporter._get_document_countcall it, so the card and the bulk title agree.dev. CHANGELOG entry under [Unreleased] Fixed.pytest tests/reporting: 148 passed.