From 294199d54233b07d97946ae0093c8728cf4bfa4a Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Tue, 11 Aug 2026 14:51:01 -0400 Subject: [PATCH] fix: abstain on translucent gradients over images, drop phantom color-mix stops Two follow-up review findings on the merge with main. A gradient leading a url() layer was treated as a resolvable surface even when its stops are translucent, so the glow and AI-palette hunts averaged wash stops (a 20% black wash reads as pure black) while the real surface blends with image pixels the engine cannot read. resolveBackgroundInfo now marks gradient-over-image unresolved unless every readable stop of the leading gradient is opaque, in which case the gradient provably covers the image and remains the scorable surface. parseGradientColorsModern predated this branch's parseGradientColors rewrite: its second regex pass re-extracted color tokens nested inside color-mix() stops that the shared parser already captures whole via balanced-paren tokens, appending ingredient colors that are never painted. The worst-case stop ratio then invented low-contrast findings against a color nobody sees. The helper is removed; all callers use the shared parser, which covers the modern syntaxes it existed for. Fixture coverage pins both: the translucent-wash-over-image glow abstains in both engines, an opaque gradient over an image still flags in the browser, and the color-mix wash case stays clean in the static engine. Each new assertion was verified to fail against the previous engine. Addresses Greptile and Cursor Bugbot review findings on PR #541. AI-assisted-by: Claude Code Co-Authored-By: Claude --- cli/engine/detect-antipatterns-browser.js | 49 ++++++++++++--------- cli/engine/rules/checks.mjs | 49 ++++++++++++--------- tests/detect-antipatterns-browser.test.mjs | 12 +++++ tests/detect-antipatterns-fixtures.test.mjs | 23 ++++++++++ tests/fixtures/antipatterns/color.html | 16 +++++++ tests/fixtures/antipatterns/glow.html | 28 ++++++++++++ 6 files changed, 135 insertions(+), 42 deletions(-) diff --git a/cli/engine/detect-antipatterns-browser.js b/cli/engine/detect-antipatterns-browser.js index 6eeaac78d..8d5bce3f6 100644 --- a/cli/engine/detect-antipatterns-browser.js +++ b/cli/engine/detect-antipatterns-browser.js @@ -3076,13 +3076,30 @@ function resolveBackgroundInfo(el, win, customPropMap) { // hides (the shipped miss: `url(photo), linear-gradient(...)` // reported low-contrast against the invisible gradient's stops). if (hasGradientOrUrl) { - const topPaintLayer = splitTopLevelCommas(bgImage).find( + const layers = splitTopLevelCommas(bgImage); + const topPaintLayer = layers.find( (layer) => /gradient\s*\(/i.test(layer) || /url\s*\(/i.test(layer), ); const gradientOnTop = !!topPaintLayer && /gradient\s*\(/i.test(topPaintLayer) && !/^\s*url\s*\(/i.test(topPaintLayer); - return { color: null, unresolved: !gradientOnTop }; + if (!gradientOnTop) return { color: null, unresolved: true }; + // Gradient on top of a url() layer: the image shows through wherever + // the gradient is not fully opaque, so a translucent wash like + // `linear-gradient(rgba(0,0,0,.2), rgba(0,0,0,.2)), url(photo)` paints + // a blend with pixels this engine cannot read. Only a gradient whose + // every readable stop is opaque provably covers the image; otherwise + // the surface is unknown — abstain rather than hand callers gradient + // stops (or a stop average) the visitor never sees unmixed. + const urlBeneath = layers.some( + (layer) => layer !== topPaintLayer && /url\s*\(/i.test(layer), + ); + if (urlBeneath) { + const topStops = parseGradientColors(topPaintLayer); + const provablyOpaque = topStops.length > 0 && topStops.every((s) => (s.a ?? 1) >= 0.99); + if (!provablyOpaque) return { color: null, unresolved: true }; + } + return { color: null, unresolved: false }; } current = current.parentElement; } @@ -3096,21 +3113,6 @@ function resolveBackground(el, win, customPropMap) { return resolveBackgroundInfo(el, win, customPropMap).color; } -// parseGradientColors (shared) reads only the legacy serializations: rgb() -// and hex stops. Browsers keep modern-space stops in computed backgroundImage -// exactly as authored — `linear-gradient(oklch(7% 0.006 95), …)` stays oklch — -// which is what every token-driven page produces. Route those through -// parseAnyColor so a gradient ground is measurable rather than invisible. -function parseGradientColorsModern(bgImage) { - if (!bgImage || !/gradient/i.test(bgImage)) return []; - const colors = parseGradientColors(bgImage); - for (const m of bgImage.matchAll(/(?:oklch|oklab|hsla?|hwb)\(\s*[^()]*\)/gi)) { - const c = parseAnyColor(m[0]); - if (c) colors.push(c); - } - return colors; -} - // Walk parents looking for a gradient background and return its color stops. // Used as a fallback when resolveBackground() returns null because the // effective background is a gradient (no single solid color to compare against). @@ -3134,7 +3136,10 @@ function resolveGradientStops(el, win, customPropMap) { if (bgImage && bgImage !== 'none' && /url\s*\(/i.test(bgImage)) return null; let stops = null; if (bgImage && bgImage !== 'none' && /gradient/i.test(bgImage)) { - const parsed = parseGradientColorsModern(bgImage); + // parseGradientColors (shared) reads modern-space stops too — oklch, + // color-mix and friends via balanced-paren token capture — so browser + // computed values that keep the authored syntax stay measurable. + const parsed = parseGradientColors(bgImage); if (parsed.length > 0) stops = parsed; } if (!stops && !DETECTOR_IS_BROWSER) { @@ -3142,7 +3147,7 @@ function resolveGradientStops(el, win, customPropMap) { const rawStyle = current.getAttribute?.('style') || ''; const bgMatch = rawStyle.match(/background(?:-image)?\s*:\s*([^;]+)/i); if (bgMatch && /gradient/i.test(bgMatch[1])) { - const parsed = parseGradientColorsModern(bgMatch[1]); + const parsed = parseGradientColors(bgMatch[1]); if (parsed.length > 0) stops = parsed; } } @@ -3936,11 +3941,13 @@ function checkElementGlowDOM(el) { if (!parentBg && !parentBgInfo.unresolved) { // Gradient background — sample its colors to determine if it's dark. // Modern-syntax parsing matters here: body-level gradients now reach this - // fallback in browser mode, and their stops usually serialize as oklch. + // fallback in browser mode, and their stops usually serialize as oklch — + // which the shared parseGradientColors reads via its color-function + // token capture. let cur = el.parentElement; while (cur && cur.nodeType === 1) { const bgImage = getComputedStyle(cur).backgroundImage || ''; - const gradColors = parseGradientColorsModern(bgImage); + const gradColors = parseGradientColors(bgImage); if (gradColors.length > 0) { // Average the gradient colors const avg = { r: 0, g: 0, b: 0 }; diff --git a/cli/engine/rules/checks.mjs b/cli/engine/rules/checks.mjs index 14e8d289f..8a5654625 100644 --- a/cli/engine/rules/checks.mjs +++ b/cli/engine/rules/checks.mjs @@ -1842,13 +1842,30 @@ function resolveBackgroundInfo(el, win, customPropMap) { // hides (the shipped miss: `url(photo), linear-gradient(...)` // reported low-contrast against the invisible gradient's stops). if (hasGradientOrUrl) { - const topPaintLayer = splitTopLevelCommas(bgImage).find( + const layers = splitTopLevelCommas(bgImage); + const topPaintLayer = layers.find( (layer) => /gradient\s*\(/i.test(layer) || /url\s*\(/i.test(layer), ); const gradientOnTop = !!topPaintLayer && /gradient\s*\(/i.test(topPaintLayer) && !/^\s*url\s*\(/i.test(topPaintLayer); - return { color: null, unresolved: !gradientOnTop }; + if (!gradientOnTop) return { color: null, unresolved: true }; + // Gradient on top of a url() layer: the image shows through wherever + // the gradient is not fully opaque, so a translucent wash like + // `linear-gradient(rgba(0,0,0,.2), rgba(0,0,0,.2)), url(photo)` paints + // a blend with pixels this engine cannot read. Only a gradient whose + // every readable stop is opaque provably covers the image; otherwise + // the surface is unknown — abstain rather than hand callers gradient + // stops (or a stop average) the visitor never sees unmixed. + const urlBeneath = layers.some( + (layer) => layer !== topPaintLayer && /url\s*\(/i.test(layer), + ); + if (urlBeneath) { + const topStops = parseGradientColors(topPaintLayer); + const provablyOpaque = topStops.length > 0 && topStops.every((s) => (s.a ?? 1) >= 0.99); + if (!provablyOpaque) return { color: null, unresolved: true }; + } + return { color: null, unresolved: false }; } current = current.parentElement; } @@ -1862,21 +1879,6 @@ function resolveBackground(el, win, customPropMap) { return resolveBackgroundInfo(el, win, customPropMap).color; } -// parseGradientColors (shared) reads only the legacy serializations: rgb() -// and hex stops. Browsers keep modern-space stops in computed backgroundImage -// exactly as authored — `linear-gradient(oklch(7% 0.006 95), …)` stays oklch — -// which is what every token-driven page produces. Route those through -// parseAnyColor so a gradient ground is measurable rather than invisible. -function parseGradientColorsModern(bgImage) { - if (!bgImage || !/gradient/i.test(bgImage)) return []; - const colors = parseGradientColors(bgImage); - for (const m of bgImage.matchAll(/(?:oklch|oklab|hsla?|hwb)\(\s*[^()]*\)/gi)) { - const c = parseAnyColor(m[0]); - if (c) colors.push(c); - } - return colors; -} - // Walk parents looking for a gradient background and return its color stops. // Used as a fallback when resolveBackground() returns null because the // effective background is a gradient (no single solid color to compare against). @@ -1900,7 +1902,10 @@ function resolveGradientStops(el, win, customPropMap) { if (bgImage && bgImage !== 'none' && /url\s*\(/i.test(bgImage)) return null; let stops = null; if (bgImage && bgImage !== 'none' && /gradient/i.test(bgImage)) { - const parsed = parseGradientColorsModern(bgImage); + // parseGradientColors (shared) reads modern-space stops too — oklch, + // color-mix and friends via balanced-paren token capture — so browser + // computed values that keep the authored syntax stay measurable. + const parsed = parseGradientColors(bgImage); if (parsed.length > 0) stops = parsed; } if (!stops && !DETECTOR_IS_BROWSER) { @@ -1908,7 +1913,7 @@ function resolveGradientStops(el, win, customPropMap) { const rawStyle = current.getAttribute?.('style') || ''; const bgMatch = rawStyle.match(/background(?:-image)?\s*:\s*([^;]+)/i); if (bgMatch && /gradient/i.test(bgMatch[1])) { - const parsed = parseGradientColorsModern(bgMatch[1]); + const parsed = parseGradientColors(bgMatch[1]); if (parsed.length > 0) stops = parsed; } } @@ -2702,11 +2707,13 @@ function checkElementGlowDOM(el) { if (!parentBg && !parentBgInfo.unresolved) { // Gradient background — sample its colors to determine if it's dark. // Modern-syntax parsing matters here: body-level gradients now reach this - // fallback in browser mode, and their stops usually serialize as oklch. + // fallback in browser mode, and their stops usually serialize as oklch — + // which the shared parseGradientColors reads via its color-function + // token capture. let cur = el.parentElement; while (cur && cur.nodeType === 1) { const bgImage = getComputedStyle(cur).backgroundImage || ''; - const gradColors = parseGradientColorsModern(bgImage); + const gradColors = parseGradientColors(bgImage); if (gradColors.length > 0) { // Average the gradient colors const avg = { r: 0, g: 0, b: 0 }; diff --git a/tests/detect-antipatterns-browser.test.mjs b/tests/detect-antipatterns-browser.test.mjs index 7a9421373..b8ab7667d 100644 --- a/tests/detect-antipatterns-browser.test.mjs +++ b/tests/detect-antipatterns-browser.test.mjs @@ -128,6 +128,18 @@ describe('detectUrl — browser-only fixtures', () => { glow.filter(g => /#10b981/i.test(g.snippet || '')).length, 0, 'offset chromatic shadow on unknown surface must not be scored', ); + // Gradient-over-image split: an opaque gradient provably covers the + // image, so the dark-background tell may score against its stops; a + // translucent wash blends with unknowable pixels (a white photo under a + // 20% black wash paints ~#cccccc, not black), so the walk abstains. + assert.ok( + glow.some(g => /Colored box-shadow glow \(#f97316\) on dark background/i.test(g.snippet || '')), + 'expected colored-glow finding under a provably opaque gradient over an image', + ); + assert.equal( + glow.filter(g => /#f43f5e/i.test(g.snippet || '')).length, 0, + 'offset chromatic shadow under a translucent wash over an image must abstain', + ); }); it('image-backed text: the overlay default pass pixel-samples the image itself', async () => { diff --git a/tests/detect-antipatterns-fixtures.test.mjs b/tests/detect-antipatterns-fixtures.test.mjs index 0b3c1768d..fb9ee76f9 100644 --- a/tests/detect-antipatterns-fixtures.test.mjs +++ b/tests/detect-antipatterns-fixtures.test.mjs @@ -255,6 +255,23 @@ describe('detectHtml — static HTML/CSS fixtures', () => { ); }); + it('color: a color-mix gradient stop never leaks its nested ingredient as a phantom surface', async () => { + // The stop paints as a 16% wash composited near-black over the dark + // wrap; the bright oklch(90% ...) nested inside the color-mix is an + // ingredient, never painted. Re-extracting nested tokens appended it as + // a phantom opaque stop, and the worst-case ratio then flagged the + // light text at ~1:1 against a color nobody sees. + const f = await detectHtml(path.join(FIXTURES, 'color.html')); + const phantom = f.filter(r => + (r.antipattern === 'low-contrast' || r.antipattern === 'gray-on-color') && + /#ded9cf/i.test(r.snippet || '') + ); + assert.equal( + phantom.length, 0, + `light text on the mixed wash must not flag: ${phantom.map(r => r.snippet).join('; ')}`, + ); + }); + it('color: white text on background-image url() ancestor is not flagged as low-contrast', async () => { const f = await detectHtml(path.join(FIXTURES, 'color.html')); // The pass column has white text on a div with background-image: url(). @@ -1077,6 +1094,12 @@ describe('detectHtml — dark glow', () => { glow.filter(g => /#10b981/i.test(g.snippet)).length, 0, 'offset chromatic shadow on unknown surface must not be scored', ); + // Translucent gradient over a url() image blends with pixels the engine + // cannot read; the wash stops must never be scored as the surface. + assert.equal( + glow.filter(g => /#f43f5e/i.test(g.snippet)).length, 0, + 'offset chromatic shadow under a translucent wash over an image must abstain', + ); }); }); diff --git a/tests/fixtures/antipatterns/color.html b/tests/fixtures/antipatterns/color.html index ee29b5c7b..58ad354f7 100644 --- a/tests/fixtures/antipatterns/color.html +++ b/tests/fixtures/antipatterns/color.html @@ -36,6 +36,15 @@ .ox-glow { background: linear-gradient(160deg, rgba(52,192,168,0.09) 0%, #141419 65%); padding: 20px; } .ox-glow p { color: #e8e6e3; font-size: 18px; } .ox-glow .muted { color: #8e8c89; font-size: 16px; } + /* Gradient stop written as color-mix with a bright nested ingredient. + The stop paints as a 16% wash of the light oklch color (composited + over the dark wrap it is near-black); the nested oklch(90% ...) is an + INGREDIENT, never painted. A parser that re-extracts nested tokens + appends it as a phantom opaque light stop and the worst-case ratio + then flags the light text at ~1:1 against a color nobody sees. */ + .mix-dark-wrap { background: #0f0f11; padding: 16px; } + .mix-glow { background: linear-gradient(160deg, color-mix(in oklab, oklch(90% 0.02 95) 16%, transparent) 0%, #141419 65%); padding: 20px; } + .mix-glow p { color: #ded9cf; font-size: 16px; } /* currentcolor surface: background-color paints with the element's own text color, which is itself a var() token here. jsdom hands both through verbatim, so the walk must resolve the token via the @@ -232,6 +241,13 @@ +

color-mix gradient stop: nested ingredient is not a surface

+
+
+

Light copy on a faint mixed wash over a dark ground stays readable

+
+
+

currentcolor surface with good contrast

Dark ink text on a bone currentcolor surface

diff --git a/tests/fixtures/antipatterns/glow.html b/tests/fixtures/antipatterns/glow.html index a91cd6910..9d1b0db83 100644 --- a/tests/fixtures/antipatterns/glow.html +++ b/tests/fixtures/antipatterns/glow.html @@ -48,6 +48,12 @@ [] for the whole element on an unresolved surface). */ .photo-context { background-image: url('/fixtures/antipatterns/missing-photo.png'); padding: 16px; border-radius: 12px; } .glow-photo-halo { box-shadow: 0 0 26px rgba(217, 70, 239, 0.5); } + /* Opaque gradient atop a url() layer: every stop is opaque, so the + gradient provably covers the image — the surface IS the dark + gradient, and the dark-background glow tell may score against it + (browser hunt; the static loop has no gradient hunt). */ + .photo-opaque-grad { background: linear-gradient(#111827, #0b1220), url('/fixtures/antipatterns/missing-photo.png'); padding: 16px; border-radius: 12px; } + .glow-photo-opaque { box-shadow: 0 6px 22px rgba(249, 115, 22, 0.5); } /* ── PASS: same dark/light backgrounds, but neutral or no glow ── */ .light-colored-shadow { box-shadow: 0 2px 4px rgba(59, 130, 246, 0.15); } @@ -76,6 +82,12 @@ /* Offset chromatic shadow under the same unreadable url() surface: the dark-background glow tell needs a surface we can read — abstain. */ .photo-offset-colored { box-shadow: 0 6px 20px rgba(16, 185, 129, 0.35); } + /* Translucent gradient atop a url() layer: the image shows through the + 20% wash, so the real surface is a blend with unknowable pixels (a + white photo composites to ~#cccccc, not black). Averaging the wash + stops as if they were the surface scored this "dark" — abstain. */ + .photo-translucent-grad { background: linear-gradient(rgba(0, 0, 0, 0.2), rgba(0, 0, 0, 0.2)), url('/fixtures/antipatterns/missing-photo.png'); padding: 16px; border-radius: 12px; } + .photo-translucent-offset { box-shadow: 0 6px 22px rgba(244, 63, 94, 0.45); } /* Offset neutral text-shadow on dark card: legibility aid, not a glow */ .dark-text-offset h4 { text-shadow: 0 1px 2px rgba(0, 0, 0, 0.6); } @@ -153,6 +165,14 @@

box-shadow: 0 0 26px rgba(217, 70, 239, 0.5)

+ +

Colored glow under an opaque gradient over an image (browser)

+
+
+

Opaque dark gradient covers the photo

+

box-shadow: 0 6px 22px rgba(249, 115, 22, 0.5)

+
+