Skip to content

fix(report): colour executive summary cards and read document_count - #474

Merged
digalaviz merged 4 commits into
awslabs:devfrom
mangeshraut712:fix/467-summary-colour-count
Nov 11, 2026
Merged

digalaviz merged 4 commits into
awslabs:devfrom
mangeshraut712:fix/467-summary-colour-count

Conversation

@mangeshraut712

Copy link
Copy Markdown
Contributor

Fixes #467

  • Drop the hasattr(self.viz_engine, '_get_performance_color') guard so float metric cards use ColorUtils.get_performance_color(value, config.color_thresholds).
  • DataExtractor.get_document_count now reads a results dict's document_count key and falls back to 1 when the value is None. SectionGenerator and HTMLReporter._get_document_count call it, so the card and the bulk title agree.
  • Tests for both, which fail on dev. CHANGELOG entry under [Unreleased] Fixed.

pytest tests/reporting: 148 passed.

@github-actions

github-actions Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

Security Scan Results

ASH Security Scan Report

  • Report generated: 2026-11-11T01:20:16+00:00
  • Time since scan: 2 minutes

Scan Metadata

  • Project: ASH
  • Scan executed: 2026-11-11T01:17:43+00:00
  • ASH version: 3.1.2

Summary

Scanner Results

The table below shows findings by scanner, with status based on severity thresholds and dependencies:

  • Severity levels:
    • Suppressed (S): Findings that have been explicitly suppressed and don't affect scanner status
    • Critical (C): Highest severity findings that require immediate attention
    • High (H): Serious findings that should be addressed soon
    • Medium (M): Moderate risk findings
    • Low (L): Lower risk findings
    • Info (I): Informational findings with minimal risk
  • Duration (Time): Time taken by the scanner to complete its execution
  • Actionable: Number of findings at or above the threshold severity level that require attention
  • Result:
    • PASSED = No findings at or above threshold
    • FAILED = Findings at or above threshold
    • MISSING = Required dependencies not available
    • SKIPPED = Scanner explicitly disabled
    • ERROR = Scanner execution error
  • Threshold: The minimum severity level that will cause a scanner to fail
    • Thresholds: ALL, LOW, MEDIUM, HIGH, CRITICAL
    • Source: Values in parentheses indicate where the threshold is set:
      • global (global_settings section in the ASH_CONFIG used)
      • config (scanner config section in the ASH_CONFIG used)
      • scanner (default configuration in the plugin, if explicitly set)
  • Statistics calculation:
    • All statistics are calculated from the final aggregated SARIF report
    • Suppressed findings are counted separately and do not contribute to actionable findings
    • Scanner status is determined by comparing actionable findings to the threshold
Scanner Suppressed Critical High Medium Low Info Actionable Result Threshold
bandit 0 6 0 0 6671 0 6 FAILED MEDIUM (global)
cdk-nag 0 0 0 0 0 0 0 PASSED MEDIUM (global)
cfn-nag 0 0 0 0 0 0 0 MISSING MEDIUM (global)
checkov 0 0 0 0 0 0 0 PASSED MEDIUM (global)
detect-secrets 0 1 0 0 0 0 1 SKIPPED MEDIUM (global)
grype 0 0 0 0 0 0 0 MISSING MEDIUM (global)
npm-audit 0 0 0 0 0 0 0 PASSED MEDIUM (global)
opengrep 0 0 0 0 0 0 0 MISSING MEDIUM (global)
semgrep 3 0 0 0 0 0 0 PASSED MEDIUM (global)
syft 0 0 0 0 0 0 0 MISSING MEDIUM (global)

Top 4 Hotspots

Files with the highest number of security findings:

Finding Count File Location
2 tests/comparators/test_anls_leaf_semantics.py
2 .ash/ash_output/converted/jupyter/examples__notebooks__Strands_Evals_FCC_Live_Agent__ipynb-converted.py
2 tests/structured_object_evaluator/test_rollup_node_semantics.py
1 tests/test_readme_examples.py

Detailed Findings

Show 7 actionable findings

Finding 1: SECRET-HEX-HIGH-ENTROPY-STRING

  • Severity: HIGH
  • Scanner: detect-secrets
  • Rule ID: SECRET-HEX-HIGH-ENTROPY-STRING
  • Location: tests/comparators/test_anls_leaf_semantics.py:89

Description:
Secret of type 'Hex High Entropy String' detected in file 'tests/comparators/test_anls_leaf_semantics.py' at line 89

Code Snippet:

Secret of type Hex High Entropy String detected

Finding 2: B310

  • Severity: HIGH
  • Scanner: bandit
  • Rule ID: B310
  • Location: .ash/ash_output/converted/jupyter/examples__notebooks__Strands_Evals_FCC_Live_Agent__ipynb-converted.py:83-85

Description:
Audit url open for permitted schemes. Allowing use of file:/ or custom schemes is often unexpected.

Code Snippet:

)
with urllib.request.urlopen(url, timeout=60) as resp:
    rows = [item["row"] for item in json.load(resp)["rows"]]

Finding 3: B108

  • Severity: HIGH
  • Scanner: bandit
  • Rule ID: B108
  • Location: tests/comparators/test_anls_leaf_semantics.py:210-212

Description:
Probable insecure usage of temp file/directory.

Code Snippet:

uuid.UUID(int=1),
            pathlib.Path("/tmp/x"),
            b"bytes",

Finding 4: B307

  • Severity: HIGH
  • Scanner: bandit
  • Rule ID: B307
  • Location: tests/structured_object_evaluator/test_rollup_node_semantics.py:1363-1369

Description:
Use of possibly insecure function - consider using safer ast.literal_eval.

Code Snippet:

def published_label(node: dict, section: str, object_lists: set) -> str:
            counts_objects = eval(  # noqa: S307
                cond.group(1),
                {},
                {"data": node, "section": section, "OBJECT_LISTS": object_lists},
            )
            return eval(unit.group(1), {}, {"counts_objects": counts_objects})  # noqa: S307

Finding 5: B307

  • Severity: HIGH
  • Scanner: bandit
  • Rule ID: B307
  • Location: tests/structured_object_evaluator/test_rollup_node_semantics.py:1368-1370

Description:
Use of possibly insecure function - consider using safer ast.literal_eval.

Code Snippet:

)
            return eval(unit.group(1), {}, {"counts_objects": counts_objects})  # noqa: S307

Finding 6: B102

  • Severity: HIGH
  • Scanner: bandit
  • Rule ID: B102
  • Location: tests/test_readme_examples.py:55-57

Description:
Use of exec detected.

Code Snippet:

with contextlib.redirect_stdout(out):
        exec(code, namespace)
    prints = [line for line in code.splitlines() if line.startswith("print(")]

Finding 7: B310

  • Severity: HIGH
  • Scanner: bandit
  • Rule ID: B310
  • Location: .ash/ash_output/converted/jupyter/examples__notebooks__Strands_Evals_FCC_Live_Agent__ipynb-converted.py:83-85

Description:
Audit url open for permitted schemes. Allowing use of file:/ or custom schemes is often unexpected.

Code Snippet:

)
with urllib.request.urlopen(url, timeout=60) as resp:
    rows = [item["row"] for item in json.load(resp)["rows"]]

Report generated by Automated Security Helper (ASH) at 2026-11-11T01:20:17+00:00

@digalaviz

Copy link
Copy Markdown
Contributor

I'm taking this review. I checked the core of it on this head: the six new tests fail on current dev and pass here, so the cards now get their threshold colour and a results dict's document_count reaches both the card and the bulk title. CI passes.

One thing for you now: since #455 merged, this conflicts with dev in CHANGELOG.md only. Both PRs add a bullet under [Unreleased] / ### Fixed, so keeping both bullets under that one heading resolves it. This branch predates the 1.0.1 release, so check the entry lands under the current [Unreleased] and not inside [1.0.1]. I'll post the full review this week.

@mangeshraut712

Copy link
Copy Markdown
Contributor Author

Thanks @digalaviz! Merged dev and resolved the conflict in CHANGELOG.md with both entries preserved under [Unreleased] / ### Fixed. Clean merge and all test suites passing.

@vawsgit vawsgit left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM!

Verified at head 7eedb8c and on a local merge with current dev (which includes #549):

  • The merge with dev is clean. _json_for_script from #549 and the html.escape(title) calls are intact.
  • Real reports match the default thresholds. A bulk ProcessEvaluation of 3 documents gives Precision 0.444 orange, Recall 1.000 green, F1 0.615 yellow, and the title "Evaluation Report - 3 Documents". A dict with document_count: 7 shows 7. document_count=None shows 1, where dev showed "None Documents".
  • Escaping holds: document_count='<script>alert(1)</script>' renders as &lt;script&gt;. Card colours come only from the fixed palette.
  • tests/reporting gives 159 passed at head and 168 on the merge. Mutations are caught: the hasattr guard restored, isinstance(value, float), the dict branch dropped, the None fallback dropped, section_generator reverted to getattr.
  • All "Done when" items in #467 are met. The CHANGELOG entry is under [Unreleased].

Non-blocking (low):

  • No test pins the _get_document_count change (html_reporter.py:309) or the not 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.000 in a threshold colour, where they used to render as 1 in #010101. The CHANGELOG does not mention it.

@digalaviz digalaviz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: None becomes 1 for a ProcessEvaluation without a count, and a results dict now reports its own document_count instead of 1. Correct, but the CHANGELOG entry only names the card.
  • The not isinstance(value, bool) check (section_generator.py:47) has no test either.

@digalaviz
digalaviz merged commit 232982e into awslabs:dev Nov 11, 2026
10 checks passed
mangeshraut712 added a commit to mangeshraut712/mangeshraut712 that referenced this pull request Nov 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HTML report Executive Summary: metric cards are never coloured, and the Documents count is wrong for a dict or a missing count

4 participants