Skip to content

fix: the HTML report lists every failed screenshot - #302

Merged
pftg merged 2 commits into
snap-diff:masterfrom
oboxodo:fix/html-report-dimension-change
Oct 12, 2026
Merged

pftg merged 2 commits into
snap-diff:masterfrom
oboxodo:fix/html-report-dimension-change

Conversation

@oboxodo

@oboxodo oboxodo commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

A screenshot whose capture changed size fails the test, but it is missing from snap_diff_report.html, and the report header counts it as passed. Underneath, any error while building a report entry did the same thing, silently unless DEBUG was set. This PR fixes both. Two commits, reviewable separately.

Before (real run, rake report:sample on master, with one resized pair added)

The test run sees four failures. The report says:

Screenshots: 3 failed, 5 passed, 8 total
entries: ["islands-map", "islands-variant", "portrait-layout"]

header-resized (80x80 → 80x60) is not listed, and it is counted among the passes. Nothing in the output says why. Only DEBUG=1 shows it:

[snap_diff] Reporter skipped 'header-resized': can't modify frozen SnapDiff::ComparisonResult:  ...

If every failure in a run is a size change, no report is written at all. A size change is the failure the README warns about ("every comparison then fails with Dimensions have changed"), so the report drops one of the most common failures. First seen on 2.0.0.beta4 (Ruby 3.4, vips); it's the same on master and with chunky_png.

Why

For a dimension mismatch, Comparison returns ComparisonResult.build_null(..., {different_dimensions: true}), and build_null freezes its result. ComparisonResult#region_area_size memoized:

def region_area_size
  @region_area_size ||= region&.size || 0
end

On a frozen instance, that ivar write raises FrozenError. HTML#failure_entry_for reads area_size for every failure, so the raise happened there. HTML#record rescued it per entry, warned only under DEBUG (993bcb9, "DEBUG-only diagnostics"), and dropped the entry. passed is total - failures.size, so the dropped entry became a pass.

The same raise breaks #to_h and #inspect on every null result: identical files, a missing image, a size change. That's why the message above ends in ...: Ruby couldn't inspect the receiver either.

Commit 1: the HTML report shows screenshots that changed size

The root: region_area_size no longer memoizes. Region#size is two multiplications, so the cache bought nothing, and the result stays frozen as build_null intends. I considered two other options:

  • Unfreeze the null result: this gives up the value object's immutability to protect a cache nobody needs.
  • Special-case the reporter only: this fixes one caller and leaves #to_h / #inspect raising for debuggers, error messages and custom reporters.

The entry: a size change says so. With the root fix alone, the entry would come back as area_size: 0, a green "pass" badge and a broken heatmap. For compare.dimensions_changed? the entry now carries the baseline, the new capture and dimensions_changed: "80x80 => 80x60". That wording is Reporters::Default#dimension_change, extracted from build_error_for_different_dimensions, so the report and the failure message say it the same way. The entry links no annotated or heatmap images. Reporters::Default#generate returns before annotating different-sized images, so none are drawn, and any .diff.png / .heatmap.diff.png on disk under that name are left over from an earlier same-size failure.

The template: say what is shown, and why.

  • Top bar: dimensions changed: 80x80 => 80x60. Sidebar badge: resized. Previously any entry without diff images got a green pass badge, although every entry in the report is a failure.
  • Heatmap view: the reason, and what to do next, instead of <img src="">:
    • a size change: "Heatmaps and annotations compare pixel by pixel, so they need both images at the same size … If the new size is intended, commit the new screenshot to accept it. If not, the layout changed, or the browser window size or pixel ratio differs from the run that recorded the baseline."
    • otherwise, the image driver produced no difference mask: "No heatmap: the image driver produced no difference mask for this comparison. The chunky_png driver never produces one; vips does." vips sets a mask on every difference it finds, so in practice this only shows under chunky_png (or a custom driver). The reporter stays driver-agnostic; in 2.1 the chunky_png sentence is one line to delete.
  • Annotated toggle: when no annotated file exists, the plain images are labelled Baseline (not annotated) / Current (not annotated), not Annotated Base / Annotated New.
report-resized-heatmap report-resized-annotated

rake report:sample keeps that header-resized pair (a.png 80x80 vs a_cropped.png 80x60), so the case stays visible to anyone working on the template.

Commit 2: a report error no longer hides the screenshot

The FrozenError was one bug. The silent drop is what made it look like a pass, and it would do the same to the next bug. Since 993bcb9, 2.0 has moved the other way: Reporting warns unconditionally ([snap_diff] Reporter X failed (…)), and so does AISimple ([snap_diff:ai] Backend failed for …). So the per-entry rescue in HTML#record now:

  1. always warns, in the same shape:
    [snap_diff] HTML report: could not render 'checkout' (FrozenError: can't modify frozen …)
  2. keeps the screenshot in the report as {name:, error:}, counted as failed (and in the total, even if the error comes before its comparison is read), because the report can't vouch for it. The page shows "could not render" and the error, says where to report it, and shows no empty images. This matters most for whoever opens the report as a CI artifact, because they never see stderr.

The rescue itself stays: a report that refuses to render because of one entry would be worse. With commit 1 reverted, the resized screenshot shows up through this path, as "could not render (FrozenError …)", instead of disappearing.

report-unrenderable-v2

The README's "Debug mode" section said DEBUG=1 makes the HTML reporter print why it skipped an entry. Nothing in lib/ reads DEBUG any more, so it now describes the always-on warning.

DiffTest's three sample cases script a Minitest::Mock comparison for exactly what verification reads. The globally registered HTML reporter reads past that script, which used to fail silently and now warns. Those three cases now run without registered reporters, using the same pattern as reporter_interplay_test.rb.

After (same run)

Screenshots: 4 failed, 4 passed, 8 total
header-resized: {"name":"header-resized","original":"…_base.png","new":"…_new.png","dimensions_changed":"80x80 => 80x60"}

Not changed

  • The failure message and the pass/fail outcome of a size change. It failed before and fails now, with the same text.
  • Entries for same-size failures (same keys, same values).
  • Reporter-level failures (Reporting.notify / finalize!) keep their existing handling. Only the per-entry rescue inside HTML#record changed.

Evidence

  • test/unit/difference_test.rb: a frozen build_null result answers region_area_size, to_h and inspect.
  • test/unit/reporters/html_reporter_test.rb:
    • a real 80x80 → 80x60 Comparison through HTML#record yields one entry with name, original, new and dimensions_changed, and finalize writes the report;
    • stale .diff.png / .base.diff.png / .heatmap.diff.png on disk are not linked;
    • an entry that raises while being built is warned about without DEBUG, kept with its error, counted (2 failed, 0 passed), and the rest of the report still renders;
    • an entry that raises before its comparison is read is counted in the total too: 1 total, 1 failed, 0 passed.
  • test/unit/reporters/ai_simple_test.rb: the hand-rolled HtmlCompare stub ("the exact surface the reporter touches") gains dimensions_changed?.
  • Mutation checks:
    • Restoring the memo turns the ComparisonResult test red with FrozenError … comparison_result.rb:64 in region_area_size.
    • Making the reporter ignore dimensions_changed? turns both size-change reporter tests red.
    • Reverting all of lib/ from commit 1 turns its three tests red, and the first reporter test fails exactly as reported: SoleItemExpectedError: no item found.
    • Restoring the DEBUG gate and the drop turns the commit 2 test red.
  • Same size-change entry under vips and chunky_png.
  • Report checked in headless Chrome with a chunky_png failure, a vips failure, a resized screenshot and an unrenderable entry: no broken images, the no-mask message only on the chunky_png entry, and the Annotated labels match the images shown in every case.
  • rake test:unit on the branch rebased onto master: 774 runs, 0 failures, and no could not render lines in the output (Ruby 3.4.11). standardrb: 178 files, no offenses.
  • Rebased onto master, which includes fix: stop blur_active_element clicking inputs, and green selenium_headless on Firefox 157 #301 (only CHANGELOG.md conflicted): full rake in a container built from the repo's Dockerfile, 815 runs, 0 failures, 1 skip, and no could not render lines.

Deliberately not here

  • The terminal message for a size change is still in the pre-feat: make the failure message legible #264 format: unlabelled absolute paths, no baseline: / actual: labels. That belongs in a separate, small PR.
  • Stale annotated images from an earlier same-size failure stay on disk after a size-change failure (Reporters::Default#generate doesn't call clean_tmp_files on that path). The report no longer links them, and I left the disk alone.
  • A null result for an image that disappeared mid-comparison (failed_by: "missing_image", a String) still raises TypeError in Comparison#dimensions_changed? and Reporters::Default#generate, which index it like a Hash. That's older than this PR. With this PR such a screenshot shows as "could not render (TypeError …)" instead of vanishing, and #to_h / #inspect work on it; fixing the TypeError belongs in its own small PR.
  • An unrenderable entry shows no images, even when they exist on disk. Rebuilding them inside the rescue risks raising again, for an entry that already says "this is a bug".

Summary by Sourcery

Keep every failed screenshot visible and accurately classified in HTML reports, including resized and unrenderable entries.

Bug Fixes:

  • Ensure screenshots with changed dimensions appear in HTML reports with their baseline, current capture, and dimension-change details instead of being omitted or counted as passes.
  • Keep report entries that fail to render, warn unconditionally, and count them as failures rather than silently dropping them.

Enhancements:

  • Improve HTML report messaging for resized and unrenderable screenshots, including clearer status badges, fallback image labels, and explanations when heatmaps are unavailable.
  • Make frozen null comparison results safe to inspect and serialize by removing unnecessary area-size memoization.

Documentation:

  • Update the README and changelog to document always-on HTML reporter warnings and visibility of resized or unrenderable screenshots.

Tests:

  • Add coverage for frozen null result readers, dimension-change report entries, stale diff-image handling, and retained report-rendering errors.
  • Prevent registered reporters from interfering with mock-based screenshot tests.

Summary by CodeRabbit

  • Bug Fixes
    • Screenshot comparisons with different image dimensions now appear in HTML reports with both images, the size change, and an explanation that heatmaps and annotations aren’t available. The report also identifies resized comparisons as failures and explains how to review them.
    • Report-rendering errors are printed with the screenshot name and error. Affected screenshots remain in the report as failures instead of being omitted or counted as passing, with a clear “could not render” status.

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no new actionable issue was found.

Summary

The PR keeps resized screenshots and report-entry errors visible in the HTML report.

  • Resized screenshots appear in the HTML report with their size change.
  • The HTML report keeps screenshots it cannot render.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Read screenshot comparison] --> B{Entry can be built?}
  B -->|No| C[Warn and keep error entry]
  B -->|Yes| D{Screenshot failed?}
  D -->|No| E[Count as passed]
  D -->|Yes| F{Image sizes changed?}
  F -->|Yes| G[Keep baseline, current image, and sizes]
  F -->|No| H[Keep images and difference metrics]
  C --> I[Count as failed and write report]
  G --> I
  H --> I
Loading

Reviews (2) · Last reviewed commit: "fix: a report error no longer hides the ..." · Reviewed by Greptile

@sourcery-ai

sourcery-ai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Reviewer's Guide

The PR fixes HTML reporting for resized screenshots by making frozen null results safely readable and adding dedicated dimension-change entries, while ensuring any per-entry rendering error is always warned about, retained as a failed report entry, and rendered without broken images.

Sequence diagram for resilient HTML screenshot reporting

sequenceDiagram
    participant Assertion
    participant HTML
    participant Comparison
    participant ComparisonResult
    participant Report

    HTML->>Assertion: compare
    Assertion-->>HTML: comparison
    HTML->>Comparison: difference
    Comparison-->>HTML: different comparison
    alt dimensions_changed?
        HTML->>Comparison: dimensions_changed?
        Comparison-->>HTML: true
        HTML->>Comparison: reporter.dimension_change
        Comparison-->>HTML: baseline size => current size
        HTML->>Report: retain resized failure entry
    else report entry raises
        HTML->>Report: retain error entry
        HTML-->>HTML: warn could not render
    else same-size failure
        HTML->>ComparisonResult: region_area_size
        ComparisonResult-->>HTML: area size
        HTML->>Report: retain failure entry
    end
    HTML->>Report: count entry as failed
Loading

Flow diagram for resized and unrenderable report entries

flowchart TD
    A[HTML#record assertions] --> B{Comparison available?}
    B -- No --> Z[Skip assertion]
    B -- Yes --> C{Comparison failed?}
    C -- No --> P[Count as passed]
    C -- Yes --> D{dimensions_changed?}
    D -- Yes --> R[Create resized entry with baseline, current, and dimensions_changed]
    R --> H[Render no heatmap or annotations]
    D -- No --> E[Build normal failure entry]
    E --> F{Entry rendering raises?}
    F -- No --> N[Render normal failure]
    F -- Yes --> W[Warn unconditionally]
    W --> X[Keep error entry and count as failed]
    H --> Q[Include in failed total]
    N --> Q
    X --> Q
    Q --> S[Write HTML report]
Loading

File-Level Changes

Change Details Files
Make frozen null comparison results safe to inspect and report dimension mismatches as explicit failures.
  • Remove memoization from region area calculation so frozen results remain readable.
  • Add dimension-change metadata and reuse the default reporter’s size wording.
  • Exclude stale annotations, heatmaps, and region metrics for resized screenshots.
  • Render resized entries with baseline/current images and explanatory no-heatmap messaging.
lib/snap_diff/comparison_result.rb
lib/snap_diff/reporters/default.rb
lib/snap_diff/reporters/html.rb
lib/snap_diff/reporters/templates/report.html.erb
scripts/generate_sample_report.rb
test/unit/difference_test.rb
test/unit/reporters/html_reporter_test.rb
test/unit/reporters/ai_simple_test.rb
Keep report-entry failures visible, failed, and diagnosable instead of silently converting them into passes.
  • Warn unconditionally when HTML entry construction raises.
  • Retain errored entries with error details and count them in totals, including failures before comparison access.
  • Render an explicit could-not-render state without broken or empty image links.
  • Update documentation to replace DEBUG-only diagnostics with always-on warnings.
lib/snap_diff/reporters/html.rb
lib/snap_diff/reporters/templates/report.html.erb
README.md
CHANGELOG.md
test/unit/reporters/html_reporter_test.rb
test/unit/diff_test.rb

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 43c396b4-6209-4b3a-bf8b-88eb741569ae

📥 Commits

Reviewing files that changed from the base of the PR and between 726255d and 29e9e21.


📒 Files selected for processing (1)
  • CHANGELOG.md

🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.



📝 Walkthrough
📝 Walkthrough

Walkthrough

The HTML reporter now retains rendering errors as failed entries and treats dimension changes as failures. The report displays baseline and current images, changed dimensions, and explanations when heatmaps or annotations are unavailable.

Changes

HTML Reporting

Layer / File(s) Summary
Reporter entries and failure accounting
lib/snap_diff/comparison_result.rb, lib/snap_diff/reporters/default.rb, lib/snap_diff/reporters/html.rb, test/unit/difference_test.rb, test/unit/diff_test.rb, test/unit/reporters/html_reporter_test.rb, test/unit/reporters/ai_simple_test.rb
Rendering errors now generate a warning and a failed entry. The reporter counts comparable assertions and records dimension-change details separately from other difference details. Tests cover error handling, dimension changes, null comparison results, and reporter isolation.
Report presentation and examples
lib/snap_diff/reporters/templates/report.html.erb, scripts/generate_sample_report.rb, CHANGELOG.md, README.md
The report identifies render errors and resized screenshots, and explains when heatmaps or annotations are unavailable. The sample report and documentation describe the updated output.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix


Merge Risk: ⚪ Minimal · up to 29e9e

No actionable issue was identified in the selected change. Merge after the reported rebase and normal CI checks.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 8 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: ensuring that every failed screenshot appears in the HTML report.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 8 files. (1 skipped: 1 unsupported.)


  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai sourcery-ai Bot 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.

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Approved.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@pftg

pftg commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

Thanks @oboxodo, this is ready to go. #301 just landed and its CHANGELOG entry sits in the same spot as yours, so the PR now conflicts with master. Could you rebase on master? Keeping both entries, with #301's first, is all it needs. I'll merge once CI is green.

oboxodo and others added 2 commits October 12, 2026 10:42
A screenshot whose capture changed size (Dimensions have changed:
1034x111 => 1040x111) was counted in the run summary but missing from
snap_diff_report.html. Comparison returns a frozen null ComparisonResult
for that case, and ComparisonResult#region_area_size memoized into an
ivar, so the reporter's read of area_size raised FrozenError. #record
rescues per assertion and only warns under DEBUG, so the entry vanished
silently, and the report header counted it as passed (passed is total
minus entries). The same raise hit #to_h and #inspect on every null
result.

region_area_size no longer memoizes: Region#size is two multiplications,
and the result stays frozen as build_null intends.

For a dimension change the entry carries the baseline, the new capture
and dimensions_changed ("WxH => WxH", worded by
Reporters::Default#dimension_change, the same as the failure message).
It links no annotated or heatmap images: none are drawn for
different-sized images, and any on disk are left over from an earlier
same-size failure.

The template says what it shows and why: the entry is badged "resized"
(it showed a green "pass" for any entry without diff images), the
heatmap view explains a missing heatmap (a size change, or an image
driver that produced no difference mask, which only chunky_png does)
and what to do next, and the Annotated toggle
labels plain images as not annotated instead of "Annotated Base/New".

Why the failure was silent is the next commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HTML#record rescues each entry so one bad entry can't sink the whole
report, but it warned only under DEBUG and dropped the entry. Because
passed is total minus entries, the dropped screenshot was counted as
passed: the size-change bug fixed in the previous commit showed up as a
silent pass, not as an error.

The rescue now always warns, in the same shape as Reporting's reporter
warnings, and keeps the screenshot as {name:, error:}. The report shows
it as "could not render" with the error and where to report it, and
counts it as failed: the report can't vouch for it. People reading the
report as a CI artifact never see stderr, so the entry is what tells
them.

The rescue itself stays: a report that refuses to render because of one
entry would be worse.

An entry that raises before its comparison is read is counted in the
total too, so failed can't exceed total. README's "Debug mode" said
DEBUG=1 shows skipped entries; nothing reads DEBUG any more, so it now
describes the always-on warning.

DiffTest's sample cases script a Minitest::Mock comparison for exactly
what verification reads; the global HTML reporter reads past that
script, which used to fail silently and now warns. Those three cases
now run without registered reporters, as reporter_interplay_test does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oboxodo
oboxodo force-pushed the fix/html-report-dimension-change branch from 726255d to 29e9e21 Compare October 12, 2026 13:45
@oboxodo

oboxodo commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on master; both CHANGELOG entries kept, #301's first. Unit suite and the full rake (815 runs) are green locally on the rebased branch.

@pftg
pftg merged commit 941d36d into snap-diff:master Oct 12, 2026
11 checks passed
@oboxodo
oboxodo deleted the fix/html-report-dimension-change branch October 12, 2026 19:24
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.

2 participants