mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-12 06:06:37 +03:00
Make em-dash-overuse an advisory rule with browser parity
Em-dashes are used legitimately by humans, so em-dash-overuse fired far too often. Reclassify it as the first advisory-tier rule: detected, but never a failure. Engine - Add `advisory: true` to the rule metadata schema (em-dash-overuse is the first). findings.mjs stamps `advisory: true` on advisory findings so every consumer can partition without a registry lookup. Rule count stays 58. - Raise the firing threshold from a flat 5 dashes to two gates: an absolute floor of 8 and a density of about one dash per 500 characters of body text. A long article that uses a few em-dashes no longer trips; a short, dash-per-clause page still does. Entity decoding (mdash, numeric, hex) is unchanged. Thresholds live in shared/constants.mjs so every engine agrees. Browser parity - The browser bundle carried a registry entry but no logic, so the overlay and extension could never flag it. Add checkEmDashOveruse / checkEmDashOveruseDOM in rules/checks.mjs (reads rendered text, no entity decoding needed), wire it into the injected page-level pass, and carry the advisory flag through serializeFindings so the overlay/extension can render it with the mildest affordance. CLI - Advisory findings print under a separate dimmed "Advisory" section, are excluded from the failure count, and never change the exit code (an advisory-only scan exits 0). JSON keeps them with `"advisory": true`. `--no-advisory` suppresses them entirely. Hook - Advisory rules are skipped by default in both the per-edit and Stop deep-pass hooks, so the hook never nags about them. Opt in with `.impeccable/config.json` -> `detector.advisoryRules: "include"`. Tests - Fixture + threshold + browser-adapter coverage; advisory-skip default and opt-in for the hook; formatFindings partitioning. The em-dash-overuse stand for a deferred copy rule in the tier tests is swapped to marketing-buzzword. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
e409bec7b5
commit
270f4d20aa
@@ -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;
|
||||
|
||||
+64
-5
@@ -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);
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 };
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user