Skip to content

Commit 0f710e7

Browse files
LarryLustigclaude
andcommitted
Tighten the visual tolerance, chosen by measurement
`maxDiffPixelRatio: 0.01` sounds small and is not. On a 1240x923 baseline it permits 11,445 pixels to differ — a whole axis label, a moved legend, a bar that changed height. It hid real changes three times: a reference line's label was repositioned and the baseline did not regenerate, so the fix looked like it had failed; 13 of 30 baselines turned out stale, one where every y-axis tick had drifted from 12/24/36/48 to 10/20/30/40/50 at 0.986% of pixels; and before that, a full change of axis label text from ISO dates to "Jan 3" passed as identical. My first instinct was 0.001. Measuring showed that was still mostly blind. Nudging every category label down two units — the kind of thing a refactor does by accident — changed nine charts by between 339 and 10,505 pixels: maxDiffPixelRatio 0.01 allows 11,445 caught 0 of 9 maxDiffPixelRatio 0.001 allows 1,144 caught 2 of 9 maxDiffPixels 100 allows 100 caught 9 of 9 So the fix is not a smaller ratio but a different shape. A ratio hands the largest images the largest blind spot, and those are the charts with the most to get wrong — the 10,505-pixel change slipped under 1% only because that baseline is big. 100 absolute pixels still absorbs an antialiasing fringe. Four consecutive runs pass on unchanged code, and the whole suite of 36 passes. The reasoning is recorded in playwright.config.ts, including the instruction to raise it deliberately if an upgrade makes it noisy rather than reaching for a ratio again. The Known gaps entry in ROADMAP is closed and CLAUDE.md's testing convention updated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2171c9c commit 0f710e7

4 files changed

Lines changed: 52 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Changed
11+
12+
- **Visual baselines now allow 100 differing pixels, absolute, instead of 1% of the image.** A
13+
ratio sounds small and is not: 1% of a 1240x923 baseline is 11,445 pixels — a whole axis label
14+
or a moved legend. It hid real changes three times, including one where every y-axis tick
15+
drifted from 12/24/36/48 to 10/20/30/40/50 at 0.986% of pixels, just under the threshold
16+
17+
Chosen by measurement rather than instinct. Against a deliberate 2-unit nudge to every category
18+
label, which changed nine charts by 339 to 10,505 pixels: the old `0.01` caught none of them,
19+
`0.001` caught two, and 100 absolute pixels caught all nine. A ratio is the wrong shape —
20+
it gives the largest images the largest blind spot, and those are the charts with the most to
21+
get wrong. Four consecutive runs pass on unchanged code
22+
1023
### Added
1124

1225
- **`DC116`** — bars that do not fit the plot are no longer silent. Bar width has a floor of one

CLAUDE.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -216,9 +216,11 @@ Helpers live in each directory's `setup.ts`.
216216
mutation tests will pass against broken behaviour.
217217
- **Do not call `requestUpdate()` in tests** after changing markup. The chart
218218
observes its own children; adding one masks a regression in that observer.
219-
- **Visual baselines: `maxDiffPixelRatio` is 0.01**, which has twice hidden real
220-
render changes. When a change should alter a chart, regenerate at zero
221-
tolerance and compare deliberately.
219+
- **Visual baselines allow 100 differing pixels**, absolute rather than a ratio.
220+
The old `maxDiffPixelRatio: 0.01` permitted 11,445 on a typical baseline and
221+
hid real changes three times. If an upgrade makes this noisy, raise it
222+
deliberately and record why in `playwright.config.ts` — do not go back to a
223+
ratio, which gives the largest charts the largest blind spot.
222224
- **The Playwright suite is split.** `charts.spec.ts` compares screenshots and
223225
runs locally only — the baselines are `-chromium-win32` and no CI runner
224226
reproduces them. It is bound into `prepublishOnly` so a release cannot skip

ROADMAP.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -363,10 +363,12 @@ trying the library for the first time.
363363
Not features — things that are wrong or thin, recorded so they are not
364364
rediscovered:
365365

366-
- **Visual tolerance is loose.** `maxDiffPixelRatio: 0.01` let a full change of
367-
axis label text (ISO dates → `Jan 3`) pass as identical. The baseline had to
368-
be deleted to force regeneration. Tightening it risks font-rendering
369-
flakiness; worth a deliberate decision either way.
366+
- ~~**Visual tolerance is loose.**~~ Fixed: `maxDiffPixelRatio: 0.01` allowed
367+
11,445 pixels to differ on a typical baseline and hid three separate real
368+
changes. Replaced with `maxDiffPixels: 100`, an absolute allowance — a ratio
369+
gives the largest images the largest blind spot. Measured against a deliberate
370+
label nudge: the old setting caught 0 of 9 affected charts, the new one 9 of 9,
371+
with four consecutive clean runs on unchanged code.
370372
- **`stage-chart.ts` at 88%** is the thinnest real coverage left, mostly in
371373
`calculateTextFit`.
372374
- **Outreach has not happened.** docs/review.md §8 argues discovery is the binding

playwright.config.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,38 @@ export default defineConfig({
8383
// Snapshot configuration
8484
expect: {
8585
toHaveScreenshot: {
86-
// Allow small pixel differences (anti-aliasing, font rendering)
87-
maxDiffPixelRatio: 0.01,
86+
/*
87+
* An absolute allowance, not a ratio.
88+
*
89+
* This was `maxDiffPixelRatio: 0.01`, which sounds small and is not: 1% of
90+
* a 1240x923 baseline is 11,445 pixels — a whole axis label, a moved
91+
* legend, a bar that changed height. It hid real changes twice. A
92+
* reference line's label was repositioned and the baseline did not
93+
* regenerate, so the fix looked like it had failed; and 13 of 30 baselines
94+
* turned out to be stale, including one where every y-axis tick had drifted
95+
* from 12/24/36/48 to 10/20/30/40/50 at 0.986% of pixels.
96+
*
97+
* Measured against a deliberate 2-unit nudge to every category label, which
98+
* changed nine charts by 339 to 10,505 pixels:
99+
*
100+
* ratio 0.01 allows 11,445 caught 0 of 9
101+
* ratio 0.001 allows 1,144 caught 2 of 9
102+
* 100 pixels allows 100 caught 9 of 9
103+
*
104+
* A ratio is the wrong shape here: it hands the largest images the largest
105+
* blind spot, and those are the charts with the most to get wrong. 100
106+
* absolute pixels still absorbs an antialiasing fringe, and four
107+
* consecutive runs pass with no code change.
108+
*
109+
* If a Playwright or Chromium upgrade makes this noisy, raise it
110+
* deliberately and say so here — do not reach for a ratio again.
111+
*/
112+
maxDiffPixels: 100,
88113
// Animation threshold
89114
animations: 'disabled',
90115
},
91116
toMatchSnapshot: {
92-
// Threshold for image comparison
93-
maxDiffPixelRatio: 0.01,
117+
maxDiffPixels: 100,
94118
},
95119
},
96120

0 commit comments

Comments
 (0)