Skip to content

fix(pretty_print): restore stdout after printer failures. - #551

Merged
sromoam merged 3 commits into
awslabs:devfrom
mangeshraut712:fix/424-printer-stdout-restoration
Nov 13, 2026
Merged

fix(pretty_print): restore stdout after printer failures.#551
sromoam merged 3 commits into
awslabs:devfrom
mangeshraut712:fix/424-printer-stdout-restoration

Conversation

@mangeshraut712

@mangeshraut712 mangeshraut712 commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Restores stdout after printer failures using context-managed redirection. File and terminal rendering now share a path, including the bulk summary in confusion-matrix file output. File-opening errors are reported separately from rendering errors.

Adds regression tests and a changelog entry. The separate HTML escaping work in #548 is outside this change.

Testing: merged current dev (7034ec0) in d0a4ee1, preserving the printer entry and all upstream changelog entries. The merge retains upstream HTML escaping. On Python 3.12.16, the dependency-enabled suite passed 3,526 tests (2 skipped, 1 xfailed); the separate Strands Evals integration suite passed 821 tests (1 xfailed). Repository-wide Ruff and whitespace checks pass. Independent AI review found no blocking merge issues. Hosted checks must validate the new head.

AI-assisted implementation, testing and review.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@mangeshraut712 mangeshraut712 changed the title Issue: Fixes #424 fix(pretty_print): restore stdout after printer failures. Nov 9, 2026
vawsgit
vawsgit approved these changes Nov 10, 2026
View reviewed changes

@vawsgit vawsgit left a comment •
edited
Loading

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 a5b79b6, with stickler imported from that checkout's src:

  • The new tests give 17 passed. With dev's pretty_print.py and these tests, 10 of 17 fail: the stdout-restore tests for print_non_matches, print_evaluation_results and print_confusion_matrix_html, plus the bulk file-output and invalid-regex tests. The full suite gives 4118 passed, 13 skipped, 1 xfailed, as the PR body says.
  • All four items of #424 are delivered: stdout is restored in the three printers, and the bulk info reaches file output.
  • Mutations are caught: dropping the bulk info from file output fails 2 tests, and removing the open-error message fails 1.
  • Nested redirects (print_evaluation_results calling print_confusion_matrix and print_non_matches) write to the file correctly. No caller in src/ passes output_file, so making render errors raise breaks no internal caller.
  • No conflict with dev. With #548, pretty_print.py merges automatically, and both PRs' tests pass on the merged file (23 passed). Only CHANGELOG.md conflicts. Whichever PR merges second keeps both entries.

Non-blocking (low):

  • ruff format --check fails on pretty_print.py:1293 (line over 88 characters). CI runs only ruff check, so CI will not catch it.
  • The body does not say Closes #424, so merging leaves #424 open.
  • The CHANGELOG entry has no bold lead sentence and no #424 link, unlike the entries next to it.
  • The early returns for unrecognizable results (print_confusion_matrix_html, print_non_matches) leave an old output file in place. That predates this PR, but the new README's "same rendering path" line overstates it.

@github-actions

github-actions Bot commented Nov 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Security Scan Results

ASH Security Scan Report

  • Report generated: 2026-11-13T16:23:35+00:00
  • Time since scan: 2 minutes

Scan Metadata

  • Project: ASH
  • Scan executed: 2026-11-13T16:20:44+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 0 0 0 6738 0 0 PASSED 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 0 0 0 0 0 0 PASSED 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)

Report generated by Automated Security Helper (ASH) at 2026-11-13T16:23:35+00:00

@sromoam

sromoam commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Hi! I merged the latest dev into this branch so it can be merged. dev now requires a new Dependency cooldown check, and this branch was created before that check existed, so it was never going to report and the PR stayed blocked. No changes to your work; the merge just brings in the new CI. Once the checks are green I'll merge it. Thanks for your patience!

@sromoam
sromoam merged commit ae01626 into awslabs:dev Nov 13, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vawsgit vawsgit vawsgit approved these changes
@sromoam sromoam Awaiting requested review from sromoam sromoam is a code owner
@digalaviz digalaviz Awaiting requested review from digalaviz digalaviz is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mangeshraut712 @sromoam @vawsgit