From 2cfd60765a0e6c95cfecc3c5a2fd1bee78ea9bab Mon Sep 17 00:00:00 2001
From: Abdul Wahab <32850166+abdulwahabone@users.noreply.github.com>
Date: Tue, 1 Sep 2026 04:20:51 +0500
Subject: [PATCH] Fix: do not flag Roboto in system font stacks (#678)

Treat the leading system face as primary so later Roboto fallbacks do not trigger overused-font, while named web-font primaries still flag.

AI-assisted merge: reviewed and executed by Codex under maintainer direction.
---
 cli/engine/detect-antipatterns-browser.js     | 37 ++++---
 .../engines/static-html/detect-html.mjs       |  6 +-
 cli/engine/rules/checks.mjs                   | 18 ++--
 cli/engine/shared/constants.mjs               | 21 +++-
 tests/detect-antipatterns-fixtures.test.mjs   | 15 +++
 tests/detect-antipatterns.test.js             | 28 ++++++
 .../fixtures/antipatterns/overused-font.html  | 97 +++++++++++++++++++
 7 files changed, 192 insertions(+), 30 deletions(-)
 create mode 100644 tests/fixtures/antipatterns/overused-font.html

diff --git a/cli/engine/detect-antipatterns-browser.js b/cli/engine/detect-antipatterns-browser.js
index 62f5b87f1..3f5abd0a8 100644
--- a/cli/engine/detect-antipatterns-browser.js
+++ b/cli/engine/detect-antipatterns-browser.js
@@ -70,13 +70,27 @@ function isBrandFontOnOwnDomain(font) {
   return allowed.some(suffix => host === suffix || host.endsWith('.' + suffix));
 }
 
-const GENERIC_FONTS = new Set([
+// Overused-font primary selection skips only CSS generics so a system stack
+// keeps the system face as primary; GENERIC_FONTS still includes platform
+// faces for design-system/serif resolution.
+const CSS_GENERIC_FONTS = new Set([
   'serif', 'sans-serif', 'monospace', 'cursive', 'fantasy',
-  'system-ui', 'ui-serif', 'ui-sans-serif', 'ui-monospace', 'ui-rounded',
-  '-apple-system', 'blinkmacsystemfont', 'segoe ui',
   'inherit', 'initial', 'unset', 'revert',
 ]);
 
+const GENERIC_FONTS = new Set([
+  ...CSS_GENERIC_FONTS,
+  'system-ui', 'ui-serif', 'ui-sans-serif', 'ui-monospace', 'ui-rounded',
+  '-apple-system', 'blinkmacsystemfont', 'segoe ui',
+]);
+
+function primaryFontFace(fontFamily, skip = CSS_GENERIC_FONTS) {
+  return String(fontFamily || '')
+    .split(',')
+    .map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase())
+    .find(f => f && !skip.has(f)) || null;
+}
+
 // WCAG large text thresholds are defined in points: 18pt normal text and
 // 14pt bold text. Browsers expose font-size in CSS pixels at 96px per inch.
 const WCAG_LARGE_TEXT_PX = 18 * (96 / 72);
@@ -1591,7 +1605,7 @@ function checkIconTile(opts) {
 function resolveSerif(fontFamily) {
   if (!fontFamily) return { primary: null, isSerif: false };
   const tokens = fontFamily.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase());
-  const primary = tokens.find(f => f && !GENERIC_FONTS.has(f)) || null;
+  const primary = primaryFontFace(fontFamily, GENERIC_FONTS);
   if (!primary) return { primary: null, isSerif: false };
   if (KNOWN_SERIF_FONTS.has(primary)) return { primary, isSerif: true };
   if (tokens.includes('serif')) return { primary, isSerif: true };
@@ -5190,8 +5204,7 @@ function checkTypography() {
     const style = getComputedStyle(el);
     const ff = style.fontFamily;
     if (!ff) continue;
-    const stack = ff.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase());
-    const primary = stack.find(f => f && !GENERIC_FONTS.has(f));
+    const primary = primaryFontFace(ff);
     if (!primary) continue;
     fontUsage.set(primary, (fontUsage.get(primary) || 0) + 1);
     totalTextElements++;
@@ -5436,8 +5449,7 @@ function checkPageTypography(doc, win) {
       if (rule.type !== 1) continue;
       const ff = rule.style?.fontFamily;
       if (!ff) continue;
-      const stack = ff.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase());
-      const primary = stack.find(f => f && !GENERIC_FONTS.has(f));
+      const primary = primaryFontFace(ff);
       if (primary) {
         fonts.add(primary);
         if (OVERUSED_FONTS.has(primary)) overusedFound.add(primary);
@@ -5456,11 +5468,10 @@ function checkPageTypography(doc, win) {
   const ffRe = /font-family\s*:\s*([^;}]+)/gi;
   let fm;
   while ((fm = ffRe.exec(html)) !== null) {
-    for (const f of fm[1].split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase())) {
-      if (f && !GENERIC_FONTS.has(f)) {
-        fonts.add(f);
-        if (OVERUSED_FONTS.has(f)) overusedFound.add(f);
-      }
+    const primary = primaryFontFace(fm[1]);
+    if (primary) {
+      fonts.add(primary);
+      if (OVERUSED_FONTS.has(primary)) overusedFound.add(primary);
     }
   }
 
diff --git a/cli/engine/engines/static-html/detect-html.mjs b/cli/engine/engines/static-html/detect-html.mjs
index b4efa84ac..51c34224b 100644
--- a/cli/engine/engines/static-html/detect-html.mjs
+++ b/cli/engine/engines/static-html/detect-html.mjs
@@ -1,7 +1,7 @@
 import fs from 'node:fs';
 import path from 'node:path';
 
-import { GENERIC_FONTS, OVERUSED_FONTS } from '../../shared/constants.mjs';
+import { OVERUSED_FONTS, primaryFontFace } from '../../shared/constants.mjs';
 import {
   checkSourceDesignSystem,
   collectStaticDesignSystemFindings,
@@ -51,9 +51,7 @@ function checkStaticPageTypography(document, window) {
   for (const el of document.querySelectorAll('p, h1, h2, h3, h4, h5, h6, li, td, th, dd, blockquote, figcaption, a, button, label, span, div')) {
     const hasText = el.childNodes.some(n => n.nodeType === 3 && n.textContent.trim().length > 0);
     if (!hasText) continue;
-    const ff = window.getComputedStyle(el).fontFamily || '';
-    const stack = ff.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase());
-    const primary = stack.find(f => f && !GENERIC_FONTS.has(f));
+    const primary = primaryFontFace(window.getComputedStyle(el).fontFamily);
     if (!primary) continue;
     fonts.add(primary);
     if (OVERUSED_FONTS.has(primary)) overusedFound.add(primary);
diff --git a/cli/engine/rules/checks.mjs b/cli/engine/rules/checks.mjs
index f15957b92..8bfb2b1df 100644
--- a/cli/engine/rules/checks.mjs
+++ b/cli/engine/rules/checks.mjs
@@ -9,6 +9,7 @@ import {
   WCAG_LARGE_BOLD_TEXT_PX,
   WCAG_LARGE_TEXT_PX,
   isBrandFontOnOwnDomain,
+  primaryFontFace,
 } from '../shared/constants.mjs';
 import {
   CSS_NAMED_COLORS,
@@ -331,7 +332,7 @@ function checkIconTile(opts) {
 function resolveSerif(fontFamily) {
   if (!fontFamily) return { primary: null, isSerif: false };
   const tokens = fontFamily.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase());
-  const primary = tokens.find(f => f && !GENERIC_FONTS.has(f)) || null;
+  const primary = primaryFontFace(fontFamily, GENERIC_FONTS);
   if (!primary) return { primary: null, isSerif: false };
   if (KNOWN_SERIF_FONTS.has(primary)) return { primary, isSerif: true };
   if (tokens.includes('serif')) return { primary, isSerif: true };
@@ -3930,8 +3931,7 @@ function checkTypography() {
     const style = getComputedStyle(el);
     const ff = style.fontFamily;
     if (!ff) continue;
-    const stack = ff.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase());
-    const primary = stack.find(f => f && !GENERIC_FONTS.has(f));
+    const primary = primaryFontFace(ff);
     if (!primary) continue;
     fontUsage.set(primary, (fontUsage.get(primary) || 0) + 1);
     totalTextElements++;
@@ -4176,8 +4176,7 @@ function checkPageTypography(doc, win) {
       if (rule.type !== 1) continue;
       const ff = rule.style?.fontFamily;
       if (!ff) continue;
-      const stack = ff.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase());
-      const primary = stack.find(f => f && !GENERIC_FONTS.has(f));
+      const primary = primaryFontFace(ff);
       if (primary) {
         fonts.add(primary);
         if (OVERUSED_FONTS.has(primary)) overusedFound.add(primary);
@@ -4196,11 +4195,10 @@ function checkPageTypography(doc, win) {
   const ffRe = /font-family\s*:\s*([^;}]+)/gi;
   let fm;
   while ((fm = ffRe.exec(html)) !== null) {
-    for (const f of fm[1].split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase())) {
-      if (f && !GENERIC_FONTS.has(f)) {
-        fonts.add(f);
-        if (OVERUSED_FONTS.has(f)) overusedFound.add(f);
-      }
+    const primary = primaryFontFace(fm[1]);
+    if (primary) {
+      fonts.add(primary);
+      if (OVERUSED_FONTS.has(primary)) overusedFound.add(primary);
     }
   }
 
diff --git a/cli/engine/shared/constants.mjs b/cli/engine/shared/constants.mjs
index b9152939a..ea9bb361a 100644
--- a/cli/engine/shared/constants.mjs
+++ b/cli/engine/shared/constants.mjs
@@ -56,13 +56,27 @@ function isBrandFontOnOwnDomain(font) {
   return allowed.some(suffix => host === suffix || host.endsWith('.' + suffix));
 }
 
-const GENERIC_FONTS = new Set([
+// Overused-font primary selection skips only CSS generics so a system stack
+// keeps the system face as primary; GENERIC_FONTS still includes platform
+// faces for design-system/serif resolution.
+const CSS_GENERIC_FONTS = new Set([
   'serif', 'sans-serif', 'monospace', 'cursive', 'fantasy',
-  'system-ui', 'ui-serif', 'ui-sans-serif', 'ui-monospace', 'ui-rounded',
-  '-apple-system', 'blinkmacsystemfont', 'segoe ui',
   'inherit', 'initial', 'unset', 'revert',
 ]);
 
+const GENERIC_FONTS = new Set([
+  ...CSS_GENERIC_FONTS,
+  'system-ui', 'ui-serif', 'ui-sans-serif', 'ui-monospace', 'ui-rounded',
+  '-apple-system', 'blinkmacsystemfont', 'segoe ui',
+]);
+
+function primaryFontFace(fontFamily, skip = CSS_GENERIC_FONTS) {
+  return String(fontFamily || '')
+    .split(',')
+    .map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase())
+    .find(f => f && !skip.has(f)) || null;
+}
+
 // WCAG large text thresholds are defined in points: 18pt normal text and
 // 14pt bold text. Browsers expose font-size in CSS pixels at 96px per inch.
 const WCAG_LARGE_TEXT_PX = 18 * (96 / 72);
@@ -104,6 +118,7 @@ export {
   BRAND_FONT_DOMAINS,
   isBrandFontOnOwnDomain,
   GENERIC_FONTS,
+  primaryFontFace,
   WCAG_LARGE_TEXT_PX,
   WCAG_LARGE_BOLD_TEXT_PX,
   EM_DASH_FLOOR,
diff --git a/tests/detect-antipatterns-fixtures.test.mjs b/tests/detect-antipatterns-fixtures.test.mjs
index da85b0653..814f259b4 100644
--- a/tests/detect-antipatterns-fixtures.test.mjs
+++ b/tests/detect-antipatterns-fixtures.test.mjs
@@ -633,6 +633,21 @@ describe('detectHtml — static HTML/CSS fixtures', () => {
     assert.equal(f.length, 0);
   });
 
+  it('overused-font: flags named primaries and skips system-stack Roboto', async () => {
+    const f = await detectHtml(path.join(FIXTURES, 'overused-font.html'));
+    const snippets = f.filter(r => r.antipattern === 'overused-font').map(r => r.snippet).join(' | ');
+    for (const font of ['inter', 'geist', 'montserrat', 'lato']) {
+      assert.match(snippets, new RegExp(`Primary font: ${font}`), `expected flag for ${font}: ${snippets}`);
+    }
+    assert.doesNotMatch(snippets, /roboto/i, `system-stack Roboto must not be primary: ${snippets}`);
+    assert.doesNotMatch(snippets, /arial/i, `system-stack Arial must not be primary: ${snippets}`);
+    assert.equal(
+      f.some(r => r.antipattern === 'flat-type-hierarchy'),
+      false,
+      `overused-font fixture should not contain incidental type findings: ${f.map(r => `${r.antipattern}:${r.snippet}`).join('; ')}`,
+    );
+  });
+
   it('design-system: flags only values outside the provided DESIGN.md tokens', async () => {
     const designSystem = normalizeDesignSystem({
       frontmatter: {
diff --git a/tests/detect-antipatterns.test.js b/tests/detect-antipatterns.test.js
index 66f6f96d7..4f0ae4cb7 100644
--- a/tests/detect-antipatterns.test.js
+++ b/tests/detect-antipatterns.test.js
@@ -674,6 +674,34 @@ describe('detectText — overused fonts', () => {
   });
 });
 
+describe('detectHtml — overused fonts system stack', () => {
+  test('Inter before a system stack still flags overused-font', async () => {
+    const page = `<!DOCTYPE html><html><head><style>
+      body { font-family: Inter, -apple-system, BlinkMacSystemFont, sans-serif; }
+      h1 { font-size: 34px; }
+      p { font-size: 15px; }
+    </style></head><body><h1>Hello</h1><p>world</p></body></html>`;
+    await withStaticFixture({ 'index.html': page }, async ({ file }) => {
+      const f = await detectHtml(file);
+      expect(f.some(r => r.antipattern === 'overused-font' && /inter/i.test(r.snippet))).toBe(true);
+    });
+  });
+
+  test('checkPageTypography regex path skips Roboto in system stack', () => {
+    const html = `<!DOCTYPE html><html><head><style>
+      body { font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif; }
+    </style></head><body><h1>Hello</h1><p>world</p></body></html>`;
+    const doc = {
+      styleSheets: [],
+      documentElement: { outerHTML: html },
+      querySelectorAll() { return []; },
+    };
+    const win = { getComputedStyle() { return { fontSize: '16px' }; } };
+    const f = checkPageTypography(doc, win);
+    expect(f.filter(r => r.id === 'overused-font')).toHaveLength(0);
+  });
+});
+
 describe('detectText — flat type hierarchy', () => {
   test('flags sizes too close together', () => {
     const page = '<!DOCTYPE html><html><style>h1{font-size:18px}h2{font-size:16px}h3{font-size:15px}p{font-size:14px}.s{font-size:13px}</style></html>';
diff --git a/tests/fixtures/antipatterns/overused-font.html b/tests/fixtures/antipatterns/overused-font.html
new file mode 100644
index 000000000..2ace348c1
--- /dev/null
+++ b/tests/fixtures/antipatterns/overused-font.html
@@ -0,0 +1,97 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <title>Overused Font — Side by Side</title>
+  <style>
+    body {
+      margin: 0;
+      padding: 24px;
+      background: #f9fafb;
+      color: #111827;
+      font: 16px/1.5 system-ui, sans-serif;
+    }
+    .grid {
+      display: grid;
+      grid-template-columns: minmax(0, 1fr) minmax(0, 1fr);
+      gap: 32px;
+      max-width: 1180px;
+      margin: 0 auto;
+    }
+    .col-label {
+      margin: 0 0 14px;
+      color: #475569;
+      font-size: 13px;
+      font-weight: 700;
+      letter-spacing: 0.08em;
+      text-transform: uppercase;
+    }
+    article { margin: 0 0 20px; }
+    h1 { margin: 0 0 24px; font-size: 48px; line-height: 1.1; }
+    h2 { margin: 0 0 8px; font-size: 22px; line-height: 1.2; }
+    p { margin: 0; font-size: 16px; color: #374151; }
+    .face-inter { font-family: Inter, sans-serif; }
+    .face-geist { font-family: Geist, sans-serif; }
+    .face-montserrat { font-family: Montserrat, sans-serif; }
+    .face-lato { font-family: Lato, sans-serif; }
+    .face-system-shorthand {
+      font: 16px/1.5 -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif;
+    }
+    .face-system-family {
+      font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif;
+    }
+    .face-system-ui { font-family: system-ui, sans-serif; }
+    .face-segoe { font-family: "Segoe UI", Roboto, sans-serif; }
+    .face-ui-sans { font-family: ui-sans-serif, Roboto, sans-serif; }
+  </style>
+</head>
+<body>
+  <h1>Overused Font</h1>
+  <div class="grid">
+    <section class="col" data-col="flag">
+      <p class="col-label">Should flag</p>
+      <article>
+        <h2>Overused Inter</h2>
+        <p class="face-inter">Inter is the primary face on this block, so overused-font should name it.</p>
+      </article>
+      <article>
+        <h2>Overused Geist</h2>
+        <p class="face-geist">Geist is the primary face on this block, so overused-font should name it.</p>
+      </article>
+      <article>
+        <h2>Overused Montserrat</h2>
+        <p class="face-montserrat">Montserrat is the primary face on this block, so overused-font should name it.</p>
+      </article>
+      <article>
+        <h2>Overused Lato</h2>
+        <p class="face-lato">Lato is the primary face on this block, so overused-font should name it.</p>
+      </article>
+    </section>
+    <section class="col" data-col="pass">
+      <p class="col-label">Should pass</p>
+      <article>
+        <h2>Canonical System Stack</h2>
+        <p class="face-system-shorthand">Roboto sits after Apple and Segoe UI faces in this stack, so it is a fallback, not the primary.</p>
+      </article>
+      <article>
+        <h2>Font Family System Stack</h2>
+        <p class="face-system-family">The same stack written as font-family must also leave Roboto unflagged.</p>
+      </article>
+      <article>
+        <h2>System UI Only</h2>
+        <p class="face-system-ui">system-ui with a generic fallback is a platform face, not an overused web font.</p>
+      </article>
+      <article>
+        <h2>Segoe UI Lead</h2>
+        <p class="face-segoe">Segoe UI in the lead position keeps Roboto as a later platform fallback.</p>
+      </article>
+      <article>
+        <h2>Ui Sans Lead</h2>
+        <p class="face-ui-sans">ui-sans-serif in the lead position keeps Roboto as a later platform fallback.</p>
+      </article>
+    </section>
+  </div>
+<script src="/js/detect-antipatterns-browser.js"></script>
+</body>
+</html>