Skip to content

Commit 636b693

Browse files
jathaydeclaude
andcommitted
fix(report): three Copilot review fixes — TTY detection + CHANGELOG accuracy
PR #20 review surfaced two distinct issues, both real: 1. Color decisions were made against the StringIO sink, not the real terminal. Each detector's print_report instantiated Report::Style.new(io: @output), and the rake task passes `output: sink` (a StringIO) so the sink can be flushed around the top/bottom Summary renders. StringIO is never a TTY, so the body of the report came out plain even on a real terminal, while the Summary (rendered directly to $stdout) was colored. Inconsistent and silently dead — exactly the failure mode TTY detection is supposed to prevent. Fix: build one Report::Style instance in the rake task bound to $stdout (the real terminal), thread it through every detector via the existing `style:` constructor parameter, and pass the same instance to Summary. Detectors keep writing to sink but make their color decisions consistently against the real output target. Audit was the one detector that didn't accept `style:` yet (every other migrated detector did) — added the parameter. 2. CHANGELOG severity table claimed inline_style and helper_recommended were errors, but SEVERITY_FOR_TYPE in audit.rb classifies them as warnings (and the rake task's summary entries match the warning classification). Updated the CHANGELOG table to match the code. Also added a note about a11y_deep: section heading is a fixed banner, but per-finding severity varies based on axe impact — critical/serious become error, moderate becomes warning, minor becomes suggestion. Full suite 498/498. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8626523 commit 636b693

3 files changed

Lines changed: 30 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,11 @@ Each detector type carries an implicit severity that drives both summary groupin
3434

3535
| Severity | Detectors |
3636
|---|---|
37-
| `error` | `raw_color`, `tailwind_arbitrary`, `inline_style`, `helper_recommended`, all 4 static `a11y` rules, `a11y_deep` (critical/serious impacts), `visual_diff` (any mismatch above threshold) |
38-
| `warning` | `stimulus orphaned`, `stimulus dead`, `missing previews`, `orphan slots`, `a11y_deep` (moderate impact) |
39-
| `suggestion` | `similar partials`, `cross-codebase patterns`, `class-itis`, `a11y_deep` (minor impact) |
37+
| `error` | `raw_color`, `tailwind_arbitrary`, all 4 static `a11y` rules (`image_alt`, `button_name`, `link_name`, `input_label`), `visual_diff` (any mismatch above threshold) |
38+
| `warning` | `inline_style`, `helper_recommended`, `stimulus orphaned`, `stimulus dead`, `missing previews`, `orphan slots` |
39+
| `suggestion` | `similar partials`, `cross-codebase patterns`, `class-itis` |
40+
41+
**`a11y_deep` is a special case:** its section heading is fixed (rendered as a single banner), but each finding is tagged at a severity derived from axe-core's `impact` field — `critical` and `serious` become `error`, `moderate` becomes `warning`, `minor` becomes `suggestion`. So a single `a11y_deep` section can contain a mix of severity tags. The summary entry for the section uses the strictest impact present (default: `:error` if any are present) so the rollup over-counts toward urgency rather than under-counts.
4042

4143
This isn't yet exposed as a configurable filter (e.g. `SEVERITY=error` to mute suggestions) — see follow-ups below.
4244

lib/guardrails/audit.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,14 @@ class Audit
7070
flood-color lighting-color stop-color
7171
].freeze
7272

73-
def initialize(root:, output: $stdout, suggest: false, format: :text, apply: false)
73+
def initialize(root:, output: $stdout, suggest: false, format: :text, apply: false, style: nil)
7474
@root = Pathname(root)
7575
@output = output
7676
@suggest = suggest
7777
@format = format
7878
@apply = apply
7979
@config = load_audit_config
80+
@style = style
8081
end
8182

8283
def run

lib/tasks/guardrails.rake

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -111,23 +111,39 @@ namespace :guardrails do
111111
# render the top-of-report summary before the per-category
112112
# details (the summary needs every detector's count). Then
113113
# write the sink + per-category sections out together.
114+
#
115+
# Important: detectors write into `sink` (StringIO) but make
116+
# ANSI-color decisions against `$stdout`. If we let each
117+
# detector instantiate its own Style bound to the sink, every
118+
# color() call would be false (StringIO isn't a TTY) and the
119+
# body of the report would print plain even on a real terminal,
120+
# while the top/bottom summary printed straight to $stdout
121+
# would still be colored. Pre-build one Style here and thread
122+
# it through every detector so the whole report tracks the
123+
# real terminal's TTY/NO_COLOR signal consistently.
114124
require "guardrails/report/summary"
125+
require "guardrails/report/style"
115126
sink = StringIO.new
127+
report_style = Guardrails::Report::Style.new(io: $stdout)
116128
violations = Guardrails::Audit.new(
117-
root: root, output: sink, suggest: suggest, apply: apply, format: :text
129+
root: root, output: sink, suggest: suggest, apply: apply, format: :text,
130+
style: report_style
118131
).run
119-
stimulus = Guardrails::StimulusAudit.new(root: root, output: sink).run
132+
stimulus = Guardrails::StimulusAudit.new(root: root, output: sink, style: report_style).run
120133
similarity_opts[:output] = sink
134+
similarity_opts[:style] = report_style
121135
similarity = Guardrails::PartialSimilarity.new(**similarity_opts).run
122-
vc = Guardrails::ViewComponentAudit.new(root: root, output: sink).run
123-
a11y = Guardrails::A11yAudit.new(root: root, output: sink).run
136+
vc = Guardrails::ViewComponentAudit.new(root: root, output: sink, style: report_style).run
137+
a11y = Guardrails::A11yAudit.new(root: root, output: sink, style: report_style).run
124138
pattern_opts[:output] = sink
139+
pattern_opts[:style] = report_style
125140
patterns = Guardrails::CrossCodebasePatterns.new(**pattern_opts).run
126141
classitis_opts[:output] = sink
142+
classitis_opts[:style] = report_style
127143
classitis = Guardrails::ClassItis.new(**classitis_opts).run
128-
a11y_deep_runner = axe_json_path ? Guardrails::A11yDeep.new(input: axe_json_path, output: sink) : nil
144+
a11y_deep_runner = axe_json_path ? Guardrails::A11yDeep.new(input: axe_json_path, output: sink, style: report_style) : nil
129145
a11y_deep = a11y_deep_runner&.run || []
130-
visual_diff_runner = visual_diff_on ? Guardrails::VisualDiff.new(root: root, output: sink) : nil
146+
visual_diff_runner = visual_diff_on ? Guardrails::VisualDiff.new(root: root, output: sink, style: report_style) : nil
131147
visual_diff = visual_diff_runner&.run || []
132148

133149
summary_entries = [
@@ -187,7 +203,7 @@ namespace :guardrails do
187203
# don't have to scroll back up after the per-detector dump.
188204
# On a long output (Patchvault has 981 findings) the bottom
189205
# recap is the load-bearing one.
190-
summary = Guardrails::Report::Summary.new(entries: summary_entries, output: $stdout)
206+
summary = Guardrails::Report::Summary.new(entries: summary_entries, output: $stdout, style: report_style)
191207
summary.render
192208
$stdout.write sink.string
193209
summary.render(recap: true)

0 commit comments

Comments
 (0)