diff --git a/cli/engine/browser/injected/index.mjs b/cli/engine/browser/injected/index.mjs index c4ed0692f..40bc1d155 100644 --- a/cli/engine/browser/injected/index.mjs +++ b/cli/engine/browser/injected/index.mjs @@ -1223,6 +1223,10 @@ if (IS_BROWSER) { type: f.type || f.id, category: ap ? ap.category : 'quality', severity: f.severity || ap?.severity || 'warning', + // Advisory findings (em-dash overuse, etc.) are surfaced but never + // treated as failures; carry the flag so the overlay/extension can + // render them with the mildest affordance and consumers can filter. + advisory: (ap && ap.advisory === true) || f.advisory === true, detail: f.detail || f.snippet, ignoreValue: f.ignoreValue || f.value || '', name: ap ? ap.name : (f.type || f.id), @@ -1541,6 +1545,17 @@ if (IS_BROWSER) { addBrowserFindings(groupMap, document.body, repeatedTextFindings); } + // Em-dash overuse (advisory): browser parity with the static/regex path. + // Reads rendered body text so it catches dashes written as HTML entities. + // serializeFindings stamps the advisory flag from the registry. + const emDashFindings = checkEmDashOveruseDOM() + .map(f => ({ type: f.id, detail: f.snippet })) + .filter(f => _ruleOk(f.type)); + if (emDashFindings.length > 0) { + pageLevelFindings.push(...emDashFindings); + addBrowserFindings(groupMap, document.body, emDashFindings); + } + const layoutFindings = checkLayout().filter(f => _ruleOk(f.type)); for (const f of layoutFindings) { const el = f.el || document.body; diff --git a/cli/engine/cli/main.mjs b/cli/engine/cli/main.mjs index e52998b7e..9f0b67105 100644 --- a/cli/engine/cli/main.mjs +++ b/cli/engine/cli/main.mjs @@ -37,9 +37,28 @@ function fileUrlToLocalPath(url) { } } -function formatFindings(findings, jsonMode) { - if (jsonMode) return JSON.stringify(findings, null, 2); +// Advisory findings are detected but never treated as failures: they list in a +// separate, visually dimmed section, are excluded from the failure count that +// drives the exit code, and carry `"advisory": true` in JSON so consumers can +// filter. Every advisory finding carries the flag (stamped by the registry via +// findings.mjs). +function isAdvisory(finding) { + return finding && finding.advisory === true; +} +function partitionAdvisory(findings) { + const primary = []; + const advisory = []; + for (const f of findings) (isAdvisory(f) ? advisory : primary).push(f); + return { primary, advisory }; +} + +// ANSI dim, when stderr is a TTY. Advisory output is chrome, so keep it quiet. +function dim(text) { + return process.stderr.isTTY ? `\x1b[2m${text}\x1b[0m` : text; +} + +function formatFindingsBody(findings) { const grouped = {}; for (const f of findings) { if (!grouped[f.file]) grouped[f.file] = []; @@ -54,7 +73,28 @@ function formatFindings(findings, jsonMode) { out.push(` → ${item.description}`); } } - out.push(`\n${formatFindingSummary(findings.length)}`); + return out; +} + +function formatAdvisorySection(advisory) { + if (!advisory || advisory.length === 0) return ''; + const lines = [`\n${dim('── Advisory (not counted as failures) ──')}`]; + for (const line of formatFindingsBody(advisory)) lines.push(dim(line)); + lines.push(dim(`\n${advisory.length} advisory note${advisory.length === 1 ? '' : 's'}. Suppress with --no-advisory.`)); + return lines.join('\n'); +} + +// Text/JSON formatter. `findings` is the full set; advisory items are separated +// out into their own section and excluded from the failure summary count. JSON +// output keeps every finding (each advisory one flagged) in a single array. +function formatFindings(findings, jsonMode) { + if (jsonMode) return JSON.stringify(findings, null, 2); + + const { primary, advisory } = partitionAdvisory(findings); + const out = [...formatFindingsBody(primary)]; + out.push(`\n${formatFindingSummary(primary.length)}`); + const advisorySection = formatAdvisorySection(advisory); + if (advisorySection) out.push(advisorySection); return out.join('\n'); } @@ -115,8 +155,14 @@ Options: ignore comments, or DESIGN.md --no-inline-ignores Do not honor in-file impeccable-disable* ignore comments --no-design-system Do not load local DESIGN.md / .impeccable/design.json context + --no-advisory Suppress advisory findings entirely (e.g. em-dash overuse) --help Show this help message +Advisory findings: + Some rules are advisory: detected and listed in a separate section, but never + counted as failures and never changing the exit code. They stay out of the + failure count so they never block automation. --no-advisory hides them. + Project config: Respects .impeccable/config.json and .impeccable/config.local.json detector settings: detector.ignoreRules, detector.ignoreFiles, detector.ignoreValues, @@ -154,6 +200,7 @@ async function detectCli() { const jsonMode = args.includes('--json'); const quietMode = args.includes('--quiet'); const helpMode = args.includes('--help'); + const noAdvisory = args.includes('--no-advisory'); // --fast (regex-only) is deprecated: since the jsdom removal, the static // HTML/CSS analysis is fast and covers every rule, so the regex-only path // only loses coverage for no real speed win. Accept the flag for back-compat @@ -365,12 +412,24 @@ async function detectCli() { allFindings = filterDetectionFindings(allFindings, detectionConfig); allFindings = filterByScopes(allFindings, scopes); + // --no-advisory drops advisory findings before any output or exit-code math. + if (noAdvisory) allFindings = allFindings.filter((f) => !isAdvisory(f)); + + // The exit code and failure count reflect non-advisory findings only. An + // advisory-only scan still prints its notes but exits 0 (a clean pass), so + // advisory rules never break CI or block automation. + const { primary, advisory } = partitionAdvisory(allFindings); if (allFindings.length > 0) { if (jsonMode) process.stdout.write(formatFindings(allFindings, true) + '\n'); - else if (quietMode) process.stderr.write(formatFindingSummary(allFindings.length) + '\n'); + else if (quietMode) { + process.stderr.write(formatFindingSummary(primary.length) + '\n'); + if (advisory.length > 0) { + process.stderr.write(dim(`${advisory.length} advisory note${advisory.length === 1 ? '' : 's'} (not counted).`) + '\n'); + } + } else process.stderr.write(formatFindings(allFindings, false) + '\n'); - process.exit(2); + process.exit(primary.length > 0 ? 2 : 0); } if (jsonMode) process.stdout.write('[]\n'); process.exit(0); diff --git a/cli/engine/engines/regex/detect-text.mjs b/cli/engine/engines/regex/detect-text.mjs index 686aae95a..882f76b42 100644 --- a/cli/engine/engines/regex/detect-text.mjs +++ b/cli/engine/engines/regex/detect-text.mjs @@ -1,4 +1,4 @@ -import { GENERIC_FONTS, OVERUSED_FONTS } from '../../shared/constants.mjs'; +import { GENERIC_FONTS, OVERUSED_FONTS, EM_DASH_FLOOR, EM_DASH_CHARS_PER_DASH } from '../../shared/constants.mjs'; import { isNeutralColor } from '../../shared/color.mjs'; import { extractGoogleFontFamilies } from '../../shared/fonts.mjs'; import { checkSourceDesignSystem } from '../../design-system.mjs'; @@ -16,6 +16,7 @@ const hasRounded = (line) => /\brounded(?:-\w+)?\b/.test(line); const hasBorderRadius = (line) => /border-radius/i.test(line); const isSafeElement = (line) => /<(?:blockquote|nav[\s>]|pre[\s>]|code[\s>]|a\s|input[\s>]|span[\s>])/i.test(line); + /** Strip HTML to plain text — drops script/style/comments/tags so * content-text analyzers don't false-positive on code or CSS. */ function stripHtmlToText(html) { @@ -306,9 +307,16 @@ const REGEX_ANALYZERS = [ const dominant = Object.entries(counts).sort((a, b) => b[1] - a[1])[0][0]; return [finding('monotonous-spacing', filePath, `~${dominant}px used ${maxCount}/${rounded.length} times (${Math.round(pct * 100)}%)`)]; }, - // Em-dash overuse: 5+ em-dashes or "--" in body text content - // (occasional em-dash use in prose is fine; the pattern fires only - // when count crosses into AI-cadence territory). + // Em-dash overuse (ADVISORY): the AI cadence tell is em-dash *saturation*, + // not the occasional dash. Humans use em-dashes legitimately, so this rule is + // advisory (surfaced separately, never a failure, hook-skipped by default) and + // its threshold is deliberately conservative. Two gates must both hold: + // 1. Absolute floor of EM_DASH_FLOOR (8) dashes — a page with a handful + // never fires, no matter how short. + // 2. Density: at least one dash per EM_DASH_CHARS_PER_DASH (500) characters + // of body text, so a long article that uses eight across several thousand + // words is left alone while a short, dash-per-clause landing page is not. + // Raised from the old flat 5-dash floor, which fired on ordinary long prose. // // stripHtmlToText drops tags but leaves character-entity escapes intact, so // a model that writes `—`, `—`, or `—` renders an em-dash @@ -322,7 +330,11 @@ const REGEX_ANALYZERS = [ let count = 0; const re = /[—]|--(?=\S)/g; while (re.exec(text) !== null) count++; - if (count < 5) return []; + if (count < EM_DASH_FLOOR) return []; + // Saturation gate: dashes must be dense in the prose, not sprinkled through + // a long document. textLength <= count * chars-per-dash means the density is + // at or above the threshold. + if (text.length > count * EM_DASH_CHARS_PER_DASH) return []; return [finding('em-dash-overuse', filePath, `${count} em-dashes in body text`)]; }, // Marketing buzzwords: SaaS phrase list diff --git a/cli/engine/findings.mjs b/cli/engine/findings.mjs index a7b0139c3..fa98dd935 100644 --- a/cli/engine/findings.mjs +++ b/cli/engine/findings.mjs @@ -6,7 +6,13 @@ function getAP(id) { function finding(id, filePath, snippet, line = 0) { const ap = getAP(id); - return { antipattern: id, name: ap.name, description: ap.description, severity: ap.severity || 'warning', category: ap.category || null, file: filePath, line, snippet }; + const base = { antipattern: id, name: ap.name, description: ap.description, severity: ap.severity || 'warning', category: ap.category || null, file: filePath, line, snippet }; + // Advisory findings are detected but reported separately and never counted as + // failures. Carry the flag on the finding so every consumer (CLI, JSON, hook) + // can partition without a registry lookup. Only stamped when true to keep the + // finding shape stable for the vast majority of rules. + if (ap.advisory === true) base.advisory = true; + return base; } export { getAP, finding }; diff --git a/cli/engine/registry/antipatterns.mjs b/cli/engine/registry/antipatterns.mjs index e319d43df..79fd04064 100644 --- a/cli/engine/registry/antipatterns.mjs +++ b/cli/engine/registry/antipatterns.mjs @@ -213,9 +213,14 @@ const ANTIPATTERNS = [ { id: 'em-dash-overuse', category: 'slop', + // Advisory: humans use em-dashes legitimately, so this rule is opt-in noise + // rather than a failure. It fires only on the AI saturation pattern, not on + // ordinary prose. Advisory findings are surfaced separately, never counted + // as failures, and skipped by the design hook unless a project opts in. + advisory: true, name: 'Em-dash overuse', description: - 'More than two em-dashes (— or --) in body copy is an AI cadence tell. Use commas, colons, periods, or parentheses instead.', + 'Em-dash saturation in body copy is an AI cadence tell. Advisory only: humans use em-dashes legitimately, so this fires only on saturation — at least 8 em-dashes (— or --) at a density near one per 500 characters of body text — never on a long article that uses a few. Prefer commas, colons, periods, or parentheses.', skillSection: 'Copy', skillGuideline: 'no em dashes', }, @@ -556,6 +561,18 @@ function getAntipattern(id) { return ANTIPATTERNS.find(rule => rule.id === id); } +// Advisory rules are detected and reported, but never treated as failures: +// the CLI lists them under a separate "Advisory" section, they do not affect +// exit codes or the failure count, and the design hook skips them by default. +// The set is derived from the registry so a rule only needs `advisory: true`. +const ADVISORY_RULE_IDS = new Set( + ANTIPATTERNS.filter(rule => rule.advisory === true).map(rule => rule.id), +); + +function isAdvisoryRule(id) { + return ADVISORY_RULE_IDS.has(id); +} + function getRulesForCategory(category) { return ANTIPATTERNS.filter(rule => rule.category === category); } @@ -585,8 +602,10 @@ export { ANTIPATTERNS, RULE_SCOPES, RULE_ENGINE_SUPPORT, + ADVISORY_RULE_IDS, getAntipattern, getRulesForCategory, getRuleEngineSupport, + isAdvisoryRule, filterByScopes, }; diff --git a/cli/engine/rules/checks.mjs b/cli/engine/rules/checks.mjs index 93161e169..5d03c9112 100644 --- a/cli/engine/rules/checks.mjs +++ b/cli/engine/rules/checks.mjs @@ -1,5 +1,7 @@ import { BORDER_SAFE_TAGS, + EM_DASH_CHARS_PER_DASH, + EM_DASH_FLOOR, GENERIC_FONTS, KNOWN_SERIF_FONTS, OVERUSED_FONTS, @@ -2594,6 +2596,33 @@ function checkNumberedSectionLabelsDOM() { return checkNumberedSectionLabels({ candidates }); } +// Em-dash overuse (ADVISORY) — pure logic shared by the browser DOM check. +// Mirrors the regex/static-HTML analyzer in engines/regex/detect-text.mjs: +// two gates (absolute floor + density) so a long article using a few dashes is +// left alone while a short, dash-per-clause page is flagged. Operates on +// already-rendered text, so no HTML-entity decoding is needed (the browser has +// resolved `—` to the literal glyph). Exported for jsdom unit tests. +function checkEmDashOveruse(text) { + const body = typeof text === 'string' ? text.replace(/\s+/g, ' ') : ''; + let count = 0; + const re = /[—]|--(?=\S)/g; + while (re.exec(body) !== null) count++; + if (count < EM_DASH_FLOOR) return []; + if (body.length > count * EM_DASH_CHARS_PER_DASH) return []; + return [{ id: 'em-dash-overuse', snippet: `${count} em-dashes in body text` }]; +} + +function checkEmDashOveruseDOM() { + const body = document.body; + if (!body) return []; + // innerText reflects rendered, visible text; fall back to textContent for + // engines (jsdom) that don't compute innerText. + const text = typeof body.innerText === 'string' && body.innerText + ? body.innerText + : (body.textContent || ''); + return checkEmDashOveruse(text); +} + function checkElementMotionDOM(el) { const tag = el.tagName.toLowerCase(); if (SAFE_TAGS.has(tag)) return []; @@ -5121,6 +5150,8 @@ export { checkNumberedSectionLabels, checkNumberedSectionLabelsFromDoc, checkNumberedSectionLabelsDOM, + checkEmDashOveruse, + checkEmDashOveruseDOM, isRepeatedTextContainer, collectRepeatedContainerTextFindings, checkRepeatedContainerTextFromDoc, diff --git a/cli/engine/shared/constants.mjs b/cli/engine/shared/constants.mjs index 917cbf504..b9152939a 100644 --- a/cli/engine/shared/constants.mjs +++ b/cli/engine/shared/constants.mjs @@ -68,6 +68,15 @@ const GENERIC_FONTS = new Set([ const WCAG_LARGE_TEXT_PX = 18 * (96 / 72); const WCAG_LARGE_BOLD_TEXT_PX = 14 * (96 / 72); +// Em-dash overuse (advisory) thresholds, shared by the regex/static-HTML +// analyzer and the browser DOM check so both fire on the same saturation +// pattern. Two gates must hold: an absolute floor of EM_DASH_FLOOR dashes, and +// a density of at least one dash per EM_DASH_CHARS_PER_DASH characters of body +// text. A long article that uses a few em-dashes is left alone; a short, +// dash-per-clause page is not. +const EM_DASH_FLOOR = 8; +const EM_DASH_CHARS_PER_DASH = 500; + // Serif faces that show up in italic-display heroes. The rule also fires when // the primary face is unknown but the stack ends in the generic `serif` token, // which catches custom/private faces with a serif fallback. @@ -97,5 +106,7 @@ export { GENERIC_FONTS, WCAG_LARGE_TEXT_PX, WCAG_LARGE_BOLD_TEXT_PX, + EM_DASH_FLOOR, + EM_DASH_CHARS_PER_DASH, KNOWN_SERIF_FONTS, }; diff --git a/cli/lib/impeccable-config.mjs b/cli/lib/impeccable-config.mjs index a0c2af6d3..c45f74208 100644 --- a/cli/lib/impeccable-config.mjs +++ b/cli/lib/impeccable-config.mjs @@ -43,7 +43,7 @@ function detectorSection(raw) { return raw && raw.detector && typeof raw.detector === 'object' && !Array.isArray(raw.detector) ? raw.detector : null; } -const DETECTOR_CONFIG_KEYS = new Set(['ignoreRules', 'ignoreFiles', 'ignoreValues', 'designSystem']); +const DETECTOR_CONFIG_KEYS = new Set(['ignoreRules', 'ignoreFiles', 'ignoreValues', 'designSystem', 'advisoryRules']); const DEFAULT_DETECTION_CONFIG = Object.freeze({ ignoreRules: [], @@ -71,6 +71,11 @@ function cloneRawDetectionConfig() { function applyDetectionConfigSource(config, raw) { if (!raw || typeof raw !== 'object') return config; + // Advisory rules are opt-in for the design hook; the CLI carries the setting + // so config round-trips (e.g. `impeccable hooks ignore-value`) preserve it. + if (raw.advisoryRules === 'include' || raw.advisoryRules === 'exclude') { + config.advisoryRules = raw.advisoryRules; + } if (raw.designSystem && typeof raw.designSystem === 'object' && !Array.isArray(raw.designSystem)) { config.designSystem = { ...config.designSystem, @@ -151,6 +156,9 @@ function normalizeDetectionConfigForWrite(config) { out.ignoreFiles = uniqueStrings(config.ignoreFiles.filter(v => typeof v === 'string' && v.trim()).map(v => v.trim())); } out.ignoreValues = normalizeIgnoreValueEntries(config?.ignoreValues || []); + if (config?.advisoryRules === 'include' || config?.advisoryRules === 'exclude') { + out.advisoryRules = config.advisoryRules; + } if (config?.designSystem && typeof config.designSystem === 'object' && !Array.isArray(config.designSystem)) { out.designSystem = { enabled: config.designSystem.enabled === false ? false : true, diff --git a/skill/scripts/hook-lib.mjs b/skill/scripts/hook-lib.mjs index bf38e3e02..5896b3242 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -16,6 +16,7 @@ * touchFile(cache, sessionId, filePath) * suppressionNotice(filePath) * filterFindings(findings, content, ext, config) + * ADVISORY_RULES / isAdvisoryFinding(finding) * IMMEDIATE_TIER_RULES / splitFindingsByTier(findings) / perEditTieringActive(config, harness) * matchConfiguredExtension(filePath, extensions) * dedupeAgainstCache(findings, cache, sessionId, filePath) @@ -126,6 +127,26 @@ export const IMMEDIATE_TIER_RULES = new Set([ 'design-system-font-size', ]); +// ── Advisory rules ──────────────────────────────────────────────────────── +// Advisory rules are opt-in noise: the CLI reports them in a separate section +// and they never count as failures. The design hook skips them entirely by +// default — in both the per-edit PostToolUse pass and the Stop deep pass — so +// the agent is never nagged about a taste call a human might make on purpose. +// A project opts back in with `.impeccable/config.json`: +// { "detector": { "advisoryRules": "include" } } +// This set is the hook's own copy of the registry's `advisory: true` rules, +// mirroring how IMMEDIATE_TIER_RULES lists rule ids inline so the hook stays +// self-contained and testable without loading the detector. Keep it in sync +// with the registry (cli/engine/registry/antipatterns.mjs). +export const ADVISORY_RULES = new Set([ + 'em-dash-overuse', +]); + +export function isAdvisoryFinding(finding) { + const id = finding && normalizeIgnoreRule(finding.antipattern); + return Boolean(id && (ADVISORY_RULES.has(id) || finding.advisory === true)); +} + export const DEFAULT_CONFIG = Object.freeze({ enabled: true, quiet: false, @@ -136,6 +157,9 @@ export const DEFAULT_CONFIG = Object.freeze({ ignoreValues: [], extensions: [], perEditRules: 'immediate', + // Advisory rules are skipped unless a project sets detector.advisoryRules to + // "include". See ADVISORY_RULES above. + advisoryRules: 'exclude', // maxFileBytes: not every generated artifact lives under a path we can // recognize. Committed browser bundles and vendored detector copies sit // next to source and run 200KB+, while genuinely authored stylesheets in @@ -293,6 +317,11 @@ function cloneDefaultConfig() { function applyDetectorConfigSource(config, raw) { if (!raw || typeof raw !== 'object') return config; + // `detector.advisoryRules: "include"` opts the hook into advisory rules + // (em-dash overuse, etc.). Any other value keeps the default "exclude". + if (raw.advisoryRules === 'include' || raw.advisoryRules === 'exclude') { + config.advisoryRules = raw.advisoryRules; + } if (raw.designSystem && typeof raw.designSystem === 'object' && !Array.isArray(raw.designSystem)) { config.designSystem = { ...config.designSystem, @@ -755,8 +784,12 @@ export function filterFindings(findings, _content, _ext, config) { if (!Array.isArray(findings) || findings.length === 0) return []; const ignoreRules = new Set((config.ignoreRules || []).map((rule) => normalizeIgnoreRule(rule))); const ignoreValues = normalizeIgnoreValueEntries(config.ignoreValues || []); + // Advisory rules are skipped by default so the hook never nags about them; + // a project opts in with detector.advisoryRules: "include". + const includeAdvisory = (config?.advisoryRules || DEFAULT_CONFIG.advisoryRules) === 'include'; return findings.filter((f) => { if (!f || typeof f !== 'object') return false; + if (!includeAdvisory && isAdvisoryFinding(f)) return false; if (ignoreRules.has(normalizeIgnoreRule(f.antipattern))) return false; if (isIgnoredFindingValue(f, ignoreValues)) return false; return true; diff --git a/tests/detect-antipatterns-fixtures.test.mjs b/tests/detect-antipatterns-fixtures.test.mjs index 8ddfebf02..a441aa852 100644 --- a/tests/detect-antipatterns-fixtures.test.mjs +++ b/tests/detect-antipatterns-fixtures.test.mjs @@ -12,8 +12,10 @@ import { fileURLToPath } from 'url'; import { detectHtml, detectText, + formatFindings, normalizeDesignSystem, } from '../cli/engine/detect-antipatterns.mjs'; +import { checkEmDashOveruse } from '../cli/engine/rules/checks.mjs'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const FIXTURES = path.join(__dirname, 'fixtures', 'antipatterns'); @@ -927,52 +929,68 @@ describe('em-dash overuse — HTML entity escapes', () => { `t` + `

A real page heading of ordinary length

${body}

`; - // Six dashes clears the 5+ threshold. Sentence fragments keep the surrounding - // prose realistic so nothing else in the pipeline objects. - const sixNamed = 'fast — cheap — honest — simple — quiet — kind — done'; - const sixNumeric = 'fast — cheap — honest — simple — quiet — kind — done'; - const sixHex = 'fast — cheap — honest — simple — quiet — kind — done'; - const sixHexUpper = 'fast — cheap — honest — simple — quiet — kind — done'; - const sixNumericPadded = 'fast — cheap — honest — simple — quiet — kind — done'; - // Three literal glyphs + three named entities render identically; the count - // must see all six. - const mixed = 'fast — cheap — honest — simple — quiet — kind — done'; + // Eight dashes clears the raised advisory floor (EM_DASH_FLOOR = 8, up from + // the old flat 5). Packed into one short paragraph they also clear the density + // gate. Sentence fragments keep the surrounding prose realistic so nothing + // else in the pipeline objects. + const eightNamed = 'fast — cheap — honest — simple — quiet — kind — bright — calm — done'; + const eightNumeric = 'fast — cheap — honest — simple — quiet — kind — bright — calm — done'; + const eightHex = 'fast — cheap — honest — simple — quiet — kind — bright — calm — done'; + const eightHexUpper = 'fast — cheap — honest — simple — quiet — kind — bright — calm — done'; + const eightNumericPadded = 'fast — cheap — honest — simple — quiet — kind — bright — calm — done'; + // Four literal glyphs + four named entities render identically; the count + // must see all eight. + const mixed = 'fast — cheap — honest — simple — quiet — kind — bright — calm — done'; const SHOULD_FLAG = { - 'named —': sixNamed, - 'numeric —': sixNumeric, - 'hex —': sixHex, - 'uppercase-hex —': sixHexUpper, - 'zero-padded decimal —': sixNumericPadded, + 'named —': eightNamed, + 'numeric —': eightNumeric, + 'hex —': eightHex, + 'uppercase-hex —': eightHexUpper, + 'zero-padded decimal —': eightNumericPadded, 'mixed literal + entity': mixed, }; + // A long paragraph carrying exactly eight dashes across several thousand + // characters of prose. Above the absolute floor, but the density gate + // (one per ~500 chars) keeps ordinary long-form writing from flagging. + const longLowDensityFiller = 'This paragraph is written in ordinary human prose that runs on for quite a while. '.repeat(60); + const longLowDensity = `a — b — c — d — e — f — g — h — end. ${longLowDensityFiller}`; + // False-positive shapes: none of these should trip the em-dash counter. const SHOULD_PASS = { - // Below the 5+ threshold: occasional em-dash entity use is legitimate prose. + // Below the floor: seven dashes on a short page is under the raised floor of 8. + 'seven dashes below floor': 'a — b — c — d — e — f — g — done, otherwise plain sentences fill the paragraph body', + // Below the floor: occasional em-dash entity use is legitimate prose. 'two entities below threshold': 'fast — cheap — done, otherwise plain sentences fill the paragraph body', + // Above the floor but below the density gate: a long human article. + 'eight dashes across a long article': longLowDensity, // En-dashes are a different character and a different job (ranges); the em-dash // rule must not decode or count them. - 'en-dash entities': 'pages 10–20 and 30–40 and 50–60 and 70–80 and 90–100 and 1–2', - 'numeric en-dash entities': 'pages 10–20 and 30–40 and 50–60 and 70–80 and 90–100 and 1–2', + 'en-dash entities': 'pages 10–20 and 30–40 and 50–60 and 70–80 and 90–100 and 1–2 and 3–4 and 5–6 and 7–8', + 'numeric en-dash entities': 'pages 10–20 and 30–40 and 50–60 and 70–80 and 90–100 and 1–2 and 3–4 and 5–6', // Double-escaped: the visible text is the literal string "—", not a dash. - 'double-escaped ampersand': 'write &mdash; and &mdash; and &mdash; and &mdash; and &mdash; and &mdash; literally', + 'double-escaped ampersand': 'write &mdash; and &mdash; and &mdash; and &mdash; and &mdash; and &mdash; and &mdash; and &mdash; literally', // Unrelated entities must never be miscounted as dashes. 'non-dash entities': 'a b © c … d & e ™ f ® g ° h § i ¶', // Ordinary hyphenated compounds are single hyphens, not the double-hyphen tell. - 'hyphenated compounds': 'state-of-the-art, well-being, high-quality, self-service, end-to-end, at-a-glance copy', + 'hyphenated compounds': 'state-of-the-art, well-being, high-quality, self-service, end-to-end, at-a-glance, day-to-day, off-the-shelf copy', }; - const emDashCount = (findings) => - findings.filter((r) => r.antipattern === 'em-dash-overuse').length; + const emDashFindings = (findings) => + findings.filter((r) => r.antipattern === 'em-dash-overuse'); for (const [label, body] of Object.entries(SHOULD_FLAG)) { it(`flags em-dash overuse spelled as ${label}`, () => { const findings = detectText(page(body), 'em-dash.html'); + const hits = emDashFindings(findings); assert.equal( - emDashCount(findings), 1, + hits.length, 1, `expected em-dash-overuse for "${label}", got: ${findings.map((r) => r.antipattern).join(', ') || 'none'}`, ); + // The rule is advisory: the finding must carry the flag so the CLI, JSON, + // and hook can partition it out of the failure set. + assert.equal(hits[0].advisory, true, `"${label}" finding should be marked advisory`); }); } @@ -980,7 +998,7 @@ describe('em-dash overuse — HTML entity escapes', () => { it(`does not flag ${label}`, () => { const findings = detectText(page(body), 'em-dash.html'); assert.equal( - emDashCount(findings), 0, + emDashFindings(findings).length, 0, `"${label}" should not flag em-dash overuse`, ); }); @@ -988,9 +1006,65 @@ describe('em-dash overuse — HTML entity escapes', () => { it('static-HTML path decodes entity em-dashes too (fixture file)', async () => { const findings = await detectHtml(path.join(FIXTURES, 'em-dash-entities.html')); + const hits = findings.filter((r) => r.antipattern === 'em-dash-overuse'); assert.equal( - findings.filter((r) => r.antipattern === 'em-dash-overuse').length, 1, + hits.length, 1, 'em-dash-entities.html should flag em-dash overuse via the static-HTML path', ); + assert.equal(hits[0].advisory, true, 'static-HTML em-dash finding should be advisory'); + }); +}); + +describe('formatFindings — advisory partitioning', () => { + const primary = { antipattern: 'side-tab', name: 'Side-tab', description: 'A primary finding.', file: 'a.css', line: 1, snippet: 'x' }; + const advisory = { antipattern: 'em-dash-overuse', name: 'Em-dash', description: 'An advisory finding.', file: 'a.html', line: 0, snippet: '8 em-dashes', advisory: true }; + + it('lists advisory findings in a separate section and excludes them from the failure count', () => { + const text = formatFindings([primary, advisory], false); + assert.match(text, /1 anti-pattern found\./); // primary count only + assert.match(text, /Advisory \(not counted as failures\)/); + assert.match(text, /em-dash-overuse/); + assert.match(text, /1 advisory note/); + }); + + it('reports zero failures for an advisory-only set but still shows the advisory section', () => { + const text = formatFindings([advisory], false); + assert.match(text, /0 anti-patterns found\./); + assert.match(text, /em-dash-overuse/); + }); + + it('keeps every finding (advisory flagged) in JSON output', () => { + const json = JSON.parse(formatFindings([primary, advisory], true)); + assert.equal(json.length, 2); + assert.equal(json.find((f) => f.antipattern === 'em-dash-overuse').advisory, true); + assert.equal(json.find((f) => f.antipattern === 'side-tab').advisory, undefined); + }); +}); + +describe('em-dash overuse — browser adapter parity (checkEmDashOveruse)', () => { + // The browser DOM check operates on already-rendered text, so it exercises + // the same two-gate logic without entity decoding. checkEmDashOveruse is the + // pure core the DOM wrapper calls. + const id = (findings) => findings.map((f) => f.id).join(','); + + it('flags eight dense em-dashes', () => { + const findings = checkEmDashOveruse('a — b — c — d — e — f — g — h — done'); + assert.equal(id(findings), 'em-dash-overuse'); + }); + + it('does not flag seven em-dashes (below the floor)', () => { + const findings = checkEmDashOveruse('a — b — c — d — e — f — g — done'); + assert.equal(findings.length, 0); + }); + + it('does not flag eight em-dashes spread across long prose (density gate)', () => { + const filler = 'This is ordinary human prose that continues at length. '.repeat(80); + const findings = checkEmDashOveruse(`a — b — c — d — e — f — g — h — end. ${filler}`); + assert.equal(findings.length, 0); + }); + + it('counts the double-hyphen em-dash substitute', () => { + const findings = checkEmDashOveruse('a--b c--d e--f g--h i--j k--l m--n o--p done'); + assert.equal(id(findings), 'em-dash-overuse'); }); }); diff --git a/tests/fixtures/antipatterns/em-dash-entities.html b/tests/fixtures/antipatterns/em-dash-entities.html index 655efcdaf..eaf8f0c74 100644 --- a/tests/fixtures/antipatterns/em-dash-entities.html +++ b/tests/fixtures/antipatterns/em-dash-entities.html @@ -10,7 +10,7 @@

The product is fast — it is also cheap — and it is honest — which matters — more than speed — or price - — in the long run. + — in the short term — and the long run — always.

diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index c50074a84..aa8e91b83 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -53,6 +53,8 @@ import { IMMEDIATE_TIER_RULES, splitFindingsByTier, perEditTieringActive, + ADVISORY_RULES, + isAdvisoryFinding, payload, extractFindingIgnoreValue, resolveProjectPlatform, @@ -416,6 +418,39 @@ describe('filterFindings()', () => { assert.deepEqual(filtered.map((f) => f.antipattern), ['gradient-text', 'overused-font']); }); + it('drops advisory-rule findings by default', () => { + const findings = [ + finding('side-tab', 1), + finding('em-dash-overuse', 2), + finding('gradient-text', 3), + ]; + const filtered = filterFindings(findings, '', '.html', { + ignoreRules: [], + limits: DEFAULT_CONFIG.limits, + }); + assert.deepEqual(filtered.map((f) => f.antipattern), ['side-tab', 'gradient-text']); + }); + + it('keeps advisory-rule findings when advisoryRules is "include"', () => { + const findings = [ + finding('side-tab', 1), + finding('em-dash-overuse', 2), + ]; + const filtered = filterFindings(findings, '', '.html', { + ignoreRules: [], + advisoryRules: 'include', + limits: DEFAULT_CONFIG.limits, + }); + assert.deepEqual(filtered.map((f) => f.antipattern), ['side-tab', 'em-dash-overuse']); + }); + + it('recognizes advisory findings by rule id or explicit flag', () => { + assert.ok(ADVISORY_RULES.has('em-dash-overuse')); + assert.equal(isAdvisoryFinding(finding('em-dash-overuse', 1)), true); + assert.equal(isAdvisoryFinding({ antipattern: 'anything', advisory: true }), true); + assert.equal(isAdvisoryFinding(finding('side-tab', 1)), false); + }); + it('does not treat source comments as hook suppression', () => { const content = [ '/* impeccable: ignore * */', @@ -3147,12 +3182,12 @@ describe('runHook() — per-edit tiering', () => { it('splitFindingsByTier partitions on IMMEDIATE_TIER_RULES', () => { const { immediate, deferred } = splitFindingsByTier([ finding('dark-glow', 1), - finding('em-dash-overuse', 2), + finding('marketing-buzzword', 2), finding('low-contrast', 3), finding('side-tab', 4), ]); assert.deepEqual(immediate.map((f) => f.antipattern), ['dark-glow', 'low-contrast']); - assert.deepEqual(deferred.map((f) => f.antipattern), ['em-dash-overuse', 'side-tab']); + assert.deepEqual(deferred.map((f) => f.antipattern), ['marketing-buzzword', 'side-tab']); for (const f of immediate) assert.ok(IMMEDIATE_TIER_RULES.has(f.antipattern)); }); @@ -3167,13 +3202,13 @@ describe('runHook() — per-edit tiering', () => { it('surfaces immediate-tier findings per edit and defers copy-tier ones', async () => { const file = write('src/Card.tsx', 'noop'); const det = fakeDetector([ - finding('em-dash-overuse', 3), + finding('marketing-buzzword', 3), finding('dark-glow', 5), ]); const r = await runHook({ stdinJson: JSON.stringify(eventFor(file)), env: {}, cwd, detector: det }); assert.match(r.stdout, /Design hook findings requiring review/); assert.match(r.stdout, /dark-glow/); - assert.doesNotMatch(r.stdout, /em-dash-overuse/); + assert.doesNotMatch(r.stdout, /marketing-buzzword/); assert.equal(r.audit.deferred, 1); const cache = readCache(cwd); @@ -3182,10 +3217,10 @@ describe('runHook() — per-edit tiering', () => { it('emits a clean ack when all findings are deferred, and still marks the file touched', async () => { const file = write('src/Copy.tsx', 'noop'); - const det = fakeDetector([finding('em-dash-overuse', 2)]); + const det = fakeDetector([finding('marketing-buzzword', 2)]); const r = await runHook({ stdinJson: JSON.stringify(eventFor(file, 'tier-deferred-only')), env: {}, cwd, detector: det }); assert.match(r.stdout, /No deterministic design-quality issues found/); - assert.doesNotMatch(r.stdout, /em-dash-overuse/); + assert.doesNotMatch(r.stdout, /marketing-buzzword/); assert.equal(r.audit.deferred, 1); // The touched-file entry is what lets the Stop deep pass find this file. @@ -3198,10 +3233,10 @@ describe('runHook() — per-edit tiering', () => { fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); fs.writeFileSync(getConfigPath(cwd), JSON.stringify({ hook: { perEditRules: 'all' } })); const file = write('src/Card.tsx', 'noop'); - const det = fakeDetector([finding('em-dash-overuse', 2)]); + const det = fakeDetector([finding('marketing-buzzword', 2)]); const r = await runHook({ stdinJson: JSON.stringify(eventFor(file, 'tier-all')), env: {}, cwd, detector: det }); assert.match(r.stdout, /Design hook findings requiring review/); - assert.match(r.stdout, /em-dash-overuse/); + assert.match(r.stdout, /marketing-buzzword/); assert.equal(r.audit.deferred, undefined); }); @@ -3213,11 +3248,33 @@ describe('runHook() — per-edit tiering', () => { toolName: 'edit', toolArgs: JSON.stringify({ path: file }), }; - const det = fakeDetector([finding('em-dash-overuse', 2)]); + const det = fakeDetector([finding('marketing-buzzword', 2)]); const r = await runHook({ stdinJson: JSON.stringify(githubEvent), env: {}, cwd, detector: det }); assert.equal(r.audit.harness, 'github'); const out = JSON.parse(r.stdout); - assert.match(out.additionalContext, /em-dash-overuse/); + assert.match(out.additionalContext, /marketing-buzzword/); + }); + + it('skips advisory findings per edit by default and never nags about them', async () => { + const file = write('src/Copy.tsx', 'noop'); + const det = fakeDetector([finding('em-dash-overuse', 3)]); + const r = await runHook({ stdinJson: JSON.stringify(eventFor(file, 'adv-skip')), env: {}, cwd, detector: det }); + // The only finding is advisory, so the file scans clean. + assert.match(r.stdout, /No deterministic design-quality issues found/); + assert.doesNotMatch(r.stdout, /em-dash-overuse/); + }); + + it('includes advisory findings per edit when detector.advisoryRules is "include"', async () => { + fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); + fs.writeFileSync(getConfigPath(cwd), JSON.stringify({ + hook: { perEditRules: 'all' }, + detector: { advisoryRules: 'include' }, + })); + const file = write('src/Copy.tsx', 'noop'); + const det = fakeDetector([finding('em-dash-overuse', 3)]); + const r = await runHook({ stdinJson: JSON.stringify(eventFor(file, 'adv-include')), env: {}, cwd, detector: det }); + assert.match(r.stdout, /Design hook findings requiring review/); + assert.match(r.stdout, /em-dash-overuse/); }); }); @@ -3257,14 +3314,14 @@ describe('runStopHook()', () => { const file = write('src/Card.tsx', 'noop'); const det = fakeDetector([ finding('dark-glow', 5), - finding('em-dash-overuse', 3), + finding('marketing-buzzword', 3), finding('side-tab', 7), ]); // Per-edit pass: surfaces dark-glow, defers the other two. const edit = await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); assert.match(edit.stdout, /dark-glow/); - assert.doesNotMatch(edit.stdout, /em-dash-overuse/); + assert.doesNotMatch(edit.stdout, /marketing-buzzword/); // Stop deep pass: surfaces exactly the deferred remainder. const stop = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); @@ -3272,7 +3329,7 @@ describe('runStopHook()', () => { assert.equal(stop.audit.emitted, true); const out = JSON.parse(stop.stdout); assert.equal(out.hookSpecificOutput.hookEventName, 'Stop'); - assert.match(out.hookSpecificOutput.additionalContext, /em-dash-overuse/); + assert.match(out.hookSpecificOutput.additionalContext, /marketing-buzzword/); assert.match(out.hookSpecificOutput.additionalContext, /side-tab/); assert.doesNotMatch(out.hookSpecificOutput.additionalContext, /dark-glow/); assert.equal(stop.emission.kind, 'stop-deep-pass'); @@ -3288,11 +3345,11 @@ describe('runStopHook()', () => { it('a second Stop fire is silent: deep-pass findings are remembered', async () => { const sid = 'stop-twice'; const file = write('src/Card.tsx', 'noop'); - const det = fakeDetector([finding('em-dash-overuse', 3)]); + const det = fakeDetector([finding('marketing-buzzword', 3)]); await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); const first = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); - assert.match(first.stdout, /em-dash-overuse/); + assert.match(first.stdout, /marketing-buzzword/); const second = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); assert.equal(second.stdout, ''); @@ -3303,15 +3360,44 @@ describe('runStopHook()', () => { const sid = 'stop-ignored'; fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); fs.writeFileSync(getConfigPath(cwd), JSON.stringify({ - detector: { ignoreRules: ['em-dash-overuse'] }, + detector: { ignoreRules: ['marketing-buzzword'] }, + })); + const file = write('src/Card.tsx', 'noop'); + const det = fakeDetector([finding('marketing-buzzword', 3)]); + + await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); + const stop = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); + assert.equal(stop.stdout, ''); + assert.equal(stop.audit.skipped, 'stop-clean'); + }); + + it('skips advisory findings in the deep pass by default', async () => { + const sid = 'stop-advisory'; + const file = write('src/Card.tsx', 'noop'); + const det = fakeDetector([finding('em-dash-overuse', 3)]); + + await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); + const stop = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); + // Silent either way: the advisory finding is dropped at the per-edit pass, so + // the file is never recorded as touched, and the deep pass has nothing to say. + assert.equal(stop.stdout, ''); + assert.ok(['stop-clean', 'no-touched-files'].includes(stop.audit.skipped)); + }); + + it('surfaces advisory findings in the deep pass when advisoryRules is "include"', async () => { + const sid = 'stop-advisory-include'; + fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); + fs.writeFileSync(getConfigPath(cwd), JSON.stringify({ + detector: { advisoryRules: 'include' }, })); const file = write('src/Card.tsx', 'noop'); const det = fakeDetector([finding('em-dash-overuse', 3)]); await runHook({ stdinJson: JSON.stringify(editEvent(file, sid)), env: {}, cwd, detector: det }); const stop = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); - assert.equal(stop.stdout, ''); - assert.equal(stop.audit.skipped, 'stop-clean'); + assert.equal(stop.audit.emitted, true); + const out = JSON.parse(stop.stdout); + assert.match(out.hookSpecificOutput.additionalContext, /em-dash-overuse/); }); it('honors kill switches and the re-entrancy guard', async () => {