diff --git a/.changeset/score-proportionality.md b/.changeset/score-proportionality.md new file mode 100644 index 000000000..4d7ce10bf --- /dev/null +++ b/.changeset/score-proportionality.md @@ -0,0 +1,25 @@ +--- +'@svelte-vitals/core': minor +'svelte-vitals': minor +'@svelte-vitals/vite': minor +--- + +Category scores now reflect **how much** is wrong, not merely whether anything is. + +A key — a route or a source file — used to start at 100 and lose a fixed number of points per failing rule. +That capped what a category could express: `architecture` is eight `info` rules, so no amount of bad code +moved it below 92, and three more scopes bottomed out above 90. It also erased magnitude, because one +finding moves a mean of N keys by `1/N`: on a large project, one finding and several hundred displayed the +same score. + +A key now scores the share of what it was measured against that is intact, weighted by severity. Every +category can reach 0, and the score moves with the number of findings. + +**Any category carrying a finding changes, most of them downward and by more than a point; a clean 100 stays 100.** `seo` and `correctness` stay within a point of their old values; `architecture`, `security` and +`performance` move further, because their scales were the most compressed. A `--min-health` gate calibrated +against the old numbers will start failing — recalibrate it against the new scale. `routes[].score` in the +JSON report changes meaning the same way. Stored baselines are unaffected, since they key on findings rather +than scores. + +Unchanged: the site-wide penalty stays in absolute points, a `critical` still caps a category at 79, and a +displayed 100 still means no finding among the checks that ran. diff --git a/docs/src/content/docs/guides/(reporting)/health-report.md b/docs/src/content/docs/guides/(reporting)/health-report.md index d5c0ff5d4..819cd63f2 100644 --- a/docs/src/content/docs/guides/(reporting)/health-report.md +++ b/docs/src/content/docs/guides/(reporting)/health-report.md @@ -15,12 +15,10 @@ Health is computed in two stages: For each active category (SEO, Performance), svelte-vitals computes an independent score: -- Every route starts at **100**. -- Each failing finding deducts points based on severity: - - `critical` → −15 per route - - `warning` → −5 per route - - `info` → −1 per route -- Deductions are taken once per (route, rule) pair — duplicates take the maximum deduction, not a sum. +- Each route scores the share of that category's checks it was measured against — weighted by severity — + that passed: no failures scores **100**, every applicable check failing scores **0**. +- Severity sets the weight a failing check carries: `critical` weighs 15, `warning` weighs 5, `info` weighs 1. +- A failing check counts once per (route, rule) pair — duplicates take the maximum severity, not a sum. - Route scores are averaged to produce the category's headline score. - If any critical finding is present, the headline score is capped at **79**. diff --git a/docs/src/content/docs/guides/(reporting)/reporters.md b/docs/src/content/docs/guides/(reporting)/reporters.md index b7e2501c4..170e3b4e1 100644 --- a/docs/src/content/docs/guides/(reporting)/reporters.md +++ b/docs/src/content/docs/guides/(reporting)/reporters.md @@ -53,7 +53,7 @@ svelte-vitals --reporter json "routes": [ { "route": "/about", // a route id, or a source file path for file-scoped rules - "score": 95, + "score": 95, // share of this route's rule inventory (by category/scope, weighted by severity) left intact "issues": [ { "id": "seo/single-h1", // the rule id diff --git a/docs/src/content/docs/ja/guides/(reporting)/health-report.md b/docs/src/content/docs/ja/guides/(reporting)/health-report.md index cf1ef93da..9bfacc82f 100644 --- a/docs/src/content/docs/ja/guides/(reporting)/health-report.md +++ b/docs/src/content/docs/ja/guides/(reporting)/health-report.md @@ -15,12 +15,10 @@ Health は 2 段階で計算されます： svelte-vitals は、アクティブなカテゴリ（SEO、パフォーマンスなど）ごとに独立したスコアを計算します： -- すべてのルートは **100** から始まります。 -- 失敗した検出結果はそれぞれ、重大度に応じてポイントを差し引きます： - - `critical` → ルートごとに −15 - - `warning` → ルートごとに −5 - - `info` → ルートごとに −1 -- 差し引きは（ルート、ルール）ペアごとに一度だけです。同じペアで重複した場合は、合計せず最大の差し引きだけを適用します。 +- 各ルートは、そのカテゴリで測定対象となったチェックのうち、重大度で重み付けした上で合格した割合をスコアとします。 + 失敗が一つも無ければ **100**、該当するチェックがすべて失敗すれば **0** になります。 +- 失敗したチェックの重みは重大度が決めます：`critical` は 15、`warning` は 5、`info` は 1 です。 +- 失敗したチェックは（ルート、ルール）ペアごとに一度だけ数えます。同じペアで重複した場合は、合計せず最大の重大度を適用します。 - ルートスコアを平均してカテゴリの見出しスコアを算出します。 - クリティカルな検出結果が存在する場合、見出しスコアは **79** にキャップされます。 diff --git a/docs/src/content/docs/ja/guides/(reporting)/reporters.md b/docs/src/content/docs/ja/guides/(reporting)/reporters.md index 429af03e1..91e104fc1 100644 --- a/docs/src/content/docs/ja/guides/(reporting)/reporters.md +++ b/docs/src/content/docs/ja/guides/(reporting)/reporters.md @@ -53,7 +53,7 @@ svelte-vitals --reporter json "routes": [ { "route": "/about", // ルート ID。ファイル単位のルールではソースファイルのパス - "score": 95, + "score": 95, // このルートが属するカテゴリ/スコープのルール一覧（重大度で重み付け）のうち、無傷で残った割合 "issues": [ { "id": "seo/single-h1", // ルール ID diff --git a/docs/superpowers/plans/2026-08-04-score-proportionality.md b/docs/superpowers/plans/2026-08-04-score-proportionality.md new file mode 100644 index 000000000..aedece1df --- /dev/null +++ b/docs/superpowers/plans/2026-08-04-score-proportionality.md @@ -0,0 +1,684 @@ +# Score proportionality Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make a category score move with how much is wrong, by scoring each key as the share of what it was +measured against that is intact, instead of subtracting fixed points from 100. + +**Architecture:** `computeScore` gains a rule inventory — the severity-weighted count of selected rules, +grouped by `(category, scope)` — which it derives itself from `selectRules(allRules, config)`, so no call +site changes. A key's score becomes `100 − (100 × failedWeight) / inventoryWeight` over the pairs observed on +that key, and the route mean moves into deficit space. `sitePenalty`, the critical cap, the flooring and +`computeHealth` are untouched. + +**Tech Stack:** TypeScript (ESM, `.js` import specifiers), vitest, oxlint + oxfmt, Astro Starlight docs. + +**Spec:** `docs/superpowers/specs/2026-08-04-score-proportionality-design.md`. Read it before Task 1. Every +number in this plan traces to it, and three of its decisions survived an adversarial review that rejected two +earlier drafts on arithmetic grounds — do not "simplify" the formula. + +## Global Constraints + +- **`packages/core/src/` is runtime-agnostic**: no `node:` imports, no I/O, no runtime-specific globals + (`packages/core/src/index.ts` states this verbatim). This change adds only pure computation. +- **The evaluation order is mandatory**: `100 − (100 × f) / i`, never `100 × (1 − f / i)`. The second form + yields `19.999999999999996` for `f = 88, i = 110` and displays 19 for a true 20. +- **The route mean is computed in deficit space**: `100 − (Σ keyDeficit) / N`, never as a mean of key scores. + The second form yields `49.99999999999999` for the two-key fixture below and displays 49 for a true 50. +- **`inventoryWeight` is `max(observedInventory, failedWeight)`**, with `inventoryWeight === 0 → keyScore 100`. + Without both, a result whose rule is not in the inventory divides by zero and `clamp(NaN)` is `NaN`. +- **The inventory is grouped by `(category, scope)` pair**, not by scope alone. Three call sites pass + multi-category result sets. +- **`DEDUCTION` values, `CRITICAL_CAP`, `Math.floor`, `computeHealth`, `sitePenalty` and the "present + categories only" semantics do not change.** +- **Comments earn their place only when they say something the code cannot** (`AGENTS.md`): a constraint, a + rejected alternative, a non-local dependency. Prefer one line over three. +- **Never name another tool, linter, plugin or product** in any doc, comment, changeset or commit message. +- **Conventional commits**, scoped by package. **A changeset is required** — `feat` is `minor`, listing + `@svelte-vitals/core`, `svelte-vitals`, `@svelte-vitals/vite`. +- **en/ja docs ship together.** + +## File Structure + +| File | Responsibility | +| ---------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | +| `packages/core/src/scoring/inventory.ts` | **new** — build the `(category, scope)` → weight map from a rule list and a config. One exported function, no dependency on `Result`. | +| `packages/core/src/scoring/score.ts` | modify `computeScore`: use the inventory, the ratio, the deficit-space mean. `computeHealth` and `scoresByCategory` untouched. | +| `packages/core/test/inventory.test.ts` | **new** — the inventory in isolation. | +| `packages/core/test/score.test.ts` | modify — existing expected values change; new cases for the ratio, the pair, the arithmetic. | +| `packages/core/test/health.test.ts` | modify — expected values change; Health's own logic does not. | +| `packages/core/test/json-report.test.ts`, `packages/core/test/unit-entry-file.test.ts`, `packages/vite/test/analyze.test.ts`, `packages/cli/test/resolve-args.test.ts` | modify — assertions on score values only. | +| `docs/src/content/docs/guides/(reporting)/reporters.md` + ja | modify — `routes[].score`'s meaning. | +| `.changeset/score-proportionality.md` | **new** | + +Splitting the inventory into its own file is deliberate: it is the piece with no `Result` dependency, it is +the piece a reviewer can check against the rule registry on its own, and keeping it out of `score.ts` stops +that file from growing a second concern. + +--- + +## Task 1: The rule inventory + +**Files:** + +- Create: `packages/core/src/scoring/inventory.ts` +- Test: `packages/core/test/inventory.test.ts` + +**Interfaces:** + +- Consumes: `Rule` (`packages/core/src/rule.ts`), `Config`, `Category`, `Scope`, `Severity` + (`packages/core/src/types.ts`), `selectRules` and `settingSeverity` (`packages/core/src/config-apply.ts`), + `allRules` (`packages/core/src/rules/index.ts`). +- Produces: + + ```ts + export type PairKey = `${Category}::${Scope}`; + export function pairKey(category: Category, scope: Scope): PairKey; + export function buildInventory(config: Config, rules?: readonly Rule[]): Map; + export function ruleScopes(rules: readonly Rule[]): Map; + ``` + + `buildInventory` defaults `rules` to `selectRules(allRules, config)`. `ruleScopes` maps a rule id to its + pair so `score.ts` can find which pair a result belongs to; it takes the same list `buildInventory` used. + +- [ ] **Step 1: Write the failing test** + +Create `packages/core/test/inventory.test.ts`: + +```ts +import { describe, it, expect } from 'vitest'; +import { buildInventory, pairKey, ruleScopes } from '../src/scoring/inventory.js'; +import { defineConfig } from '../src/types.js'; +import { allRules } from '../src/rules/index.js'; +import type { Rule } from '../src/rule.js'; + +const rule = (id: string, category: Rule['category'], scope: Rule['scope'], severity: Rule['severity']) => + ({ id, category, scope, severity, title: id, rationale: '', check: async () => [] }) as unknown as Rule; + +describe('buildInventory', () => { + it('sums DEDUCTION per (category, scope) pair', () => { + const rules = [ + rule('a/one', 'architecture', 'component', 'info'), + rule('a/two', 'architecture', 'component', 'warning'), + rule('p/one', 'performance', 'route', 'critical') + ]; + const inv = buildInventory(defineConfig({}), rules); + expect(inv.get(pairKey('architecture', 'component'))).toBe(6); + expect(inv.get(pairKey('performance', 'route'))).toBe(15); + expect(inv.get(pairKey('performance', 'component'))).toBeUndefined(); + }); + + it('drops a rule turned off and counts a rule whose severity is overridden', () => { + const rules = [ + rule('a/one', 'architecture', 'component', 'info'), + rule('a/two', 'architecture', 'component', 'info') + ]; + const config = defineConfig({ rules: { 'a/one': 'off', 'a/two': 'critical' } }); + const inv = buildInventory(config, rules); + expect(inv.get(pairKey('architecture', 'component'))).toBe(15); + }); + + it('defaults to the selected registry', () => { + // Eight architecture rules, all info, is what makes the old model bottom out at 92. + const inv = buildInventory(defineConfig({})); + const architecture = allRules.filter((r) => r.category === 'architecture'); + expect(inv.get(pairKey('architecture', 'component'))).toBe(architecture.length); + }); + + it('maps a rule id to its pair', () => { + const rules = [rule('a/one', 'architecture', 'component', 'info')]; + expect(ruleScopes(rules).get('a/one')).toBe(pairKey('architecture', 'component')); + expect(ruleScopes(rules).get('nope')).toBeUndefined(); + }); +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run from `packages/core`: `../../node_modules/.bin/vitest run test/inventory.test.ts` +Expected: FAIL — cannot resolve `../src/scoring/inventory.js`. + +- [ ] **Step 3: Write the implementation** + +Create `packages/core/src/scoring/inventory.ts`: + +```ts +import type { Category, Config, Scope, Severity } from '../types.js'; +import type { Rule } from '../rule.js'; +import { selectRules, settingSeverity } from '../config-apply.js'; +import { allRules } from '../rules/index.js'; + +const DEDUCTION: Record = { critical: 15, warning: 5, info: 1 }; + +export type PairKey = `${Category}::${Scope}`; + +export function pairKey(category: Category, scope: Scope): PairKey { + return `${category}::${scope}`; +} + +/** A rule's severity as configured, matching how `selectRules` reads `config.rules`. */ +function severityOf(rule: Rule, config: Config): Severity { + const setting = settingSeverity(config.rules[rule.id]); + return setting !== undefined && setting !== 'off' ? setting : rule.severity; +} + +/** + * Total severity weight per `(category, scope)` pair — the denominator a key of that pair is measured + * against. Defaults to the selected registry so `computeScore` needs no new argument; the parameter exists + * for tests and for scoring against a rule set that is not the registry. + */ +export function buildInventory( + config: Config, + rules: readonly Rule[] = selectRules(allRules, config) +): Map { + const out = new Map(); + for (const rule of rules) { + const key = pairKey(rule.category, rule.scope); + out.set(key, (out.get(key) ?? 0) + DEDUCTION[severityOf(rule, config)]); + } + return out; +} + +/** Rule id to its pair, so a result can be attributed to the inventory entry it was measured against. */ +export function ruleScopes(rules: readonly Rule[]): Map { + return new Map(rules.map((r) => [r.id, pairKey(r.category, r.scope)])); +} +``` + +> **Superseded (2026-08-04):** this `severityOf` restores an `off` rule's own severity, which contradicts the +> "drops a rule turned off" test above (that test's inventory of 15 requires the `off` rule contribute +> nothing, not its own 1). The shipped `severityOf` returns `undefined` for an `off` rule instead, and +> `buildInventory` skips it — see `packages/core/src/scoring/inventory.ts` and its test. + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `../../node_modules/.bin/vitest run test/inventory.test.ts` +Expected: PASS, 4 tests. + +- [ ] **Step 5: Confirm the registry default is what the spec measured** + +Run from `packages/core`: + +```bash +../../node_modules/.bin/vitest run test/inventory.test.ts -t 'defaults to the selected registry' +``` + +Expected: PASS. If the architecture count is not 8, the registry has changed since the spec was written — +**stop and report it** rather than adjusting the number, because the spec's motivating table depends on it. + +- [ ] **Step 6: Typecheck and lint** + +Run from `packages/core`: `../../node_modules/.bin/tsc --noEmit` +Run from the repo root: `node_modules/.bin/oxlint .` and `node_modules/.bin/oxfmt --check .` +Expected: all clean. + +- [ ] **Step 7: Commit** + +```bash +git add packages/core/src/scoring/inventory.ts packages/core/test/inventory.test.ts +git commit -m "feat(core): add the per-(category, scope) rule inventory" +``` + +--- + +## Task 2: The ratio model in `computeScore` + +**Files:** + +- Modify: `packages/core/src/scoring/score.ts` (the body of `computeScore`, lines 36–95) +- Test: `packages/core/test/score.test.ts` (append; existing cases are re-baselined in Task 3) + +**Interfaces:** + +- Consumes: `buildInventory`, `ruleScopes`, `pairKey`, `PairKey` from Task 1. +- Produces: `ScoreOptions` gains `rules?: readonly Rule[]`. `ScoreResult` and `ScoreModel` keep their current + fields and meanings — `score`, `rawScore`, `scoreModel.routeAverage`, `scoreModel.sitePenalty`, + `scoreModel.criticalCap`. + +- [ ] **Step 1: Write the failing tests** + +Append to `packages/core/test/score.test.ts`. The helpers `pass` and `fail` already exist at the top of that +file; do not redefine them. + +```ts +import type { Rule } from '../src/rule.js'; + +const r = (id: string, category: Rule['category'], scope: Rule['scope'], severity: Rule['severity']) => + ({ id, category, scope, severity, title: id, rationale: '', check: async () => [] }) as unknown as Rule; + +// Nine weight in one pair — the shape that makes the arithmetic below checkable by hand. +const PERF = [ + r('p/i1', 'performance', 'component', 'info'), + r('p/i2', 'performance', 'component', 'info'), + r('p/i3', 'performance', 'component', 'info'), + r('p/i4', 'performance', 'component', 'info'), + r('p/w1', 'performance', 'component', 'warning') +]; + +describe('computeScore — proportional model', () => { + const config = defineConfig({}); + + it('scores a key as the share of its pair that is intact', () => { + // failedWeight 5 of inventory 9 -> 100 - 500/9 = 44.44…, floored once at the category. + const results = [fail('p/w1', 'src/A.svelte', 'warning')]; + const { score } = computeScore(results, config, { rules: PERF }); + expect(score).toBe(44); + }); + + it('lets a key reach 0 when everything in its pair fails', () => { + const results = PERF.map((rule) => fail(rule.id, 'src/A.svelte', rule.severity as 'warning' | 'info')); + expect(computeScore(results, config, { rules: PERF, applyCriticalCap: false }).score).toBe(0); + }); + + it('distinguishes one affected key from many', () => { + // The reported symptom: under the old model both displayed 99. + const keys = Array.from({ length: 585 }, (_, i) => `src/${i}.svelte`); + const one = keys.map((k, i) => (i === 0 ? fail('p/i1', k, 'info') : pass('p/i1', k))); + const many = keys.map((k, i) => (i < 276 ? fail('p/i1', k, 'info') : pass('p/i1', k))); + const a = computeScore(one, config, { rules: PERF }).score; + const b = computeScore(many, config, { rules: PERF }).score; + expect(a).toBe(99); + expect(b).toBeLessThan(a); + }); + + it('sums the inventory over every pair observed on a key', () => { + // One seo route warning beside a passing performance route rule: 100 - 500/(5+5) = 50, + // where the seo pair alone would give 100 - 500/5 = 0. + const rules = [r('seo/x', 'seo', 'route', 'warning'), r('perf/y', 'performance', 'route', 'warning')]; + const results = [fail('seo/x', '/a', 'warning'), pass('perf/y', '/a')]; + expect(computeScore(results, defineConfig({}), { rules }).score).toBe(50); + }); + + it('keeps an integral score integral', () => { + // 100 - (100*88)/110 is exactly 20; 100 * (1 - 88/110) is 19.999999999999996. + const rules = [ + r('s/c1', 'seo', 'route', 'critical'), + r('s/c2', 'seo', 'route', 'critical'), + ...Array.from({ length: 14 }, (_, i) => r(`s/w${i}`, 'seo', 'route', 'warning')), + ...Array.from({ length: 10 }, (_, i) => r(`s/i${i}`, 'seo', 'route', 'info')) + ]; + // 2 criticals (30) + 11 warnings (55) + 3 infos (3) = 88, against an inventory of 110. + const results = [ + fail('s/c1', '/a', 'critical'), + fail('s/c2', '/a', 'critical'), + ...Array.from({ length: 11 }, (_, i) => fail(`s/w${i}`, '/a', 'warning')), + ...Array.from({ length: 3 }, (_, i) => fail(`s/i${i}`, '/a', 'info')) + ]; + expect(computeScore(results, defineConfig({}), { rules, applyCriticalCap: false }).score).toBe(20); + }); + + it('keeps an integral mean integral across keys', () => { + // Two keys, deficits 300/9 and 600/9, true mean exactly 50. A mean of key scores gives + // 49.99999999999999 and displays 49. + const results = [ + fail('p/i1', 'src/A.svelte', 'info'), + fail('p/i2', 'src/A.svelte', 'info'), + fail('p/i3', 'src/A.svelte', 'info'), + fail('p/w1', 'src/B.svelte', 'warning'), + fail('p/i1', 'src/B.svelte', 'info') + ]; + expect(computeScore(results, config, { rules: PERF }).score).toBe(50); + }); + + it('scores 0, not NaN, for a penalized result whose rule is not in the inventory', () => { + const results = [fail('ghost/rule', 'src/A.svelte', 'warning')]; + const { score } = computeScore(results, config, { rules: PERF, applyCriticalCap: false }); + expect(Number.isFinite(score)).toBe(true); + expect(score).toBe(0); + }); + + it('scores 100 for a key whose only results come from rules outside the inventory', () => { + const results = [pass('ghost/rule', 'src/A.svelte')]; + expect(computeScore(results, config, { rules: PERF }).score).toBe(100); + }); + + it('narrowing the rule set to one category leaves that category unchanged', () => { + const mixed = [...PERF, r('seo/x', 'seo', 'route', 'warning')]; + const results = [fail('p/w1', 'src/A.svelte', 'warning'), pass('p/i1', 'src/A.svelte')]; + expect(computeScore(results, config, { rules: mixed }).score).toBe( + computeScore(results, config, { rules: PERF }).score + ); + }); + + it('keeps a category with two scopes from merging them', () => { + // A component key must not be measured against route-scoped rules it can never trigger. + // Merged, the inventory would be 5 + 45 and the key would score 90 instead of 0. + const rules = [ + r('p/comp', 'performance', 'component', 'warning'), + ...Array.from({ length: 9 }, (_, i) => r(`p/route${i}`, 'performance', 'route', 'warning')) + ]; + const results = [fail('p/comp', 'src/A.svelte', 'warning')]; + expect(computeScore(results, config, { rules, applyCriticalCap: false }).score).toBe(0); + }); + + it('scores 100 when nothing is penalized', () => { + const results = [pass('p/i1', 'src/A.svelte'), pass('p/w1', 'src/B.svelte')]; + expect(computeScore(results, config, { rules: PERF }).score).toBe(100); + }); + + it('orders severities within one pair', () => { + const one = (id: string, sev: 'critical' | 'warning' | 'info') => + computeScore([fail(id, 'src/A.svelte', sev)], config, { + rules: [ + r('x/c', 'security', 'component', 'critical'), + r('x/w', 'security', 'component', 'warning'), + r('x/i', 'security', 'component', 'info') + ], + applyCriticalCap: false + }).score; + expect(one('x/c', 'critical')).toBeLessThan(one('x/w', 'warning')); + expect(one('x/w', 'warning')).toBeLessThan(one('x/i', 'info')); + }); +}); +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run from `packages/core`: `../../node_modules/.bin/vitest run test/score.test.ts -t 'proportional model'` +Expected: FAIL — `rules` is not a valid `ScoreOptions` property (a type error), and the values are the old +model's. + +- [ ] **Step 3: Replace the scoring body** + +In `packages/core/src/scoring/score.ts`, add the imports: + +```ts +import type { Rule } from '../rule.js'; +import { selectRules } from '../config-apply.js'; +import { allRules } from '../rules/index.js'; +import { buildInventory, ruleScopes, type PairKey } from './inventory.js'; +``` + +`buildInventory` defaults its rule list the same way, but `ruleScopes` needs the identical list, so +`computeScore` resolves it once and passes it to both rather than letting the two disagree. + +Extend `ScoreOptions`: + +```ts +export interface ScoreOptions { + applyCriticalCap?: boolean; + /** The rules that ran. Defaults to the selected registry; supplied by tests and custom rule sets. */ + rules?: readonly Rule[]; +} +``` + +Replace the body of `computeScore` from `const routeResults = …` down to and including the loop that writes +`routeScores`, with: + +```ts +const routeResults = results.filter((r) => r.route !== undefined); +const projectResults = results.filter((r) => r.route === undefined); + +const rules = options.rules ?? selectRules(allRules, config); +const inventory = buildInventory(config, rules); +const pairOf = ruleScopes(rules); + +let anyCritical = false; + +// Per key: the pairs it was measured against, and the weight that failed. One deduction per +// (key, rule id) — the max among duplicates — exactly as before; only the divisor is new. +const observed = new Map>(); +const ruleMax = new Map>(); +for (const r of routeResults) { + const key = r.route as string; + if (!observed.has(key)) observed.set(key, new Set()); + const pair = pairOf.get(r.id); + if (pair !== undefined) observed.get(key)!.add(pair); + if (!isPenalized(r.detection, config.treatDynamicAs)) continue; + const sev = effectiveSeverity(r, config); + if (sev === 'critical') anyCritical = true; + let perRule = ruleMax.get(key); + if (!perRule) ruleMax.set(key, (perRule = new Map())); + const prev = perRule.get(r.id) ?? 0; + if (DEDUCTION[sev] > prev) perRule.set(r.id, DEDUCTION[sev]); +} + +// Deficit space, as `computeHealth` already works: a mean of key scores computes +// 49.99999999999999 for a true 50 on two keys of deficit 300/9 and 600/9. +let totalDeficit = 0; +for (const [key, pairs] of observed) { + let failed = 0; + for (const d of ruleMax.get(key)?.values() ?? []) failed += d; + let inventoryWeight = 0; + for (const p of pairs) inventoryWeight += inventory.get(p) ?? 0; + // `max` covers the two cases where a result outweighs its own inventory: `treatDynamicAs: 'warn'` + // promotes a result's severity without changing its rule's, and a rule absent from the inventory + // observes no pair. Both would otherwise divide by zero, and `clamp(NaN)` is `NaN`. + inventoryWeight = Math.max(inventoryWeight, failed); + // `100 - (100 * f) / i`, never `100 * (1 - f / i)`: the latter gives 19.999999999999996 for + // f = 88, i = 110 and displays 19 for a true 20. + totalDeficit += inventoryWeight === 0 ? 0 : (100 * failed) / inventoryWeight; +} + +const keyCount = observed.size; +const rawRouteAverage = keyCount === 0 ? 100 : 100 - totalDeficit / keyCount; +const routeAverage = Math.floor(rawRouteAverage); +``` + +Delete the now-dead `routeScores` map, the `scores` array, the old `rawRouteAverage` line and its comment +block, and the `for (const [route, perRule] of routeRuleMax)` loop. Leave everything from +`// One deduction per project rule id` onward exactly as it is. + +- [ ] **Step 4: Rewrite the stale comment** + +The comment above the old `routeAverage` claims no epsilon is needed because "every route score is an +integer". Key scores are no longer integers. Replace it with the two-line comment already shown in Step 3 +above `totalDeficit`; do not leave the old text anywhere in the file. A false premise in a comment about +floating point is how this file's previous arithmetic bugs survived review. + +- [ ] **Step 5: Run the new tests** + +Run from `packages/core`: `../../node_modules/.bin/vitest run test/score.test.ts -t 'proportional model'` +Expected: PASS, 12 tests. Older cases in the same file will now fail — that is expected and is Task 3's work. + +- [ ] **Step 6: Prove each guard is load-bearing** + +Run each mutation, confirm the named test fails, then restore and confirm green. Report every result. + +| mutation | must fail | +| ------------------------------------------- | ------------------------------------------------------------------------------- | +| `100 * (1 - failed / inventoryWeight)` | `keeps an integral score integral` | +| mean of key scores instead of deficit space | `keeps an integral mean integral across keys` | +| drop the `Math.max` | `scores 0, not NaN, …` | +| drop the `inventoryWeight === 0` guard | `scores 100 for a key whose only results come from rules outside the inventory` | +| sum by `scope` alone, ignoring `category` | `sums the inventory over every pair observed on a key` | +| sum by `category` alone, ignoring `scope` | `keeps a category with two scopes from merging them` | + +If a mutation leaves the suite green, the test is wrong, not the implementation. + +- [ ] **Step 7: Typecheck** + +Run from `packages/core`: `../../node_modules/.bin/tsc --noEmit` +Expected: clean. + +- [ ] **Step 8: Commit** + +```bash +git add packages/core/src/scoring/score.ts packages/core/test/score.test.ts +git commit -m "feat(core): score a key by the share of its checks that pass" +``` + +--- + +## Task 3: Re-baseline every test that asserts a score + +**Files:** + +- Modify: `packages/core/test/score.test.ts`, `packages/core/test/health.test.ts`, + `packages/core/test/json-report.test.ts`, `packages/core/test/unit-entry-file.test.ts`, + `packages/vite/test/analyze.test.ts`, `packages/cli/test/resolve-args.test.ts` + +**Interfaces:** + +- Consumes: `computeScore` as changed in Task 2. Nothing new is produced. + +- [ ] **Step 1: See the full damage** + +Run each package's suite and collect every failure: + +```bash +(cd packages/core && ../../node_modules/.bin/vitest run) +(cd packages/cli && ../../node_modules/.bin/vitest run) +(cd packages/vite && ../../node_modules/.bin/vitest run) +``` + +Expected: failures confined to assertions on score, health, or `routes[].score` values. **A failure anywhere +else is a real regression — stop and report it** rather than editing the expectation. + +- [ ] **Step 2: Update each expectation from the formula, not from the old value** + +For each failing assertion, recompute the expected number by hand: +`keyScore = 100 − (100 × failedWeight) / inventoryWeight`, then +`categoryScore = Math.floor(100 − (Σ keyDeficit) / N − sitePenalty)`. + +The inventories these tests hit come from the real registry, so use `buildInventory(config)` in a scratch +check rather than guessing. **Do not paste the value the test runner reports** — that makes the test agree +with whatever was implemented instead of with the design. + +Where a test asserts a specific number only to prove that "something was deducted", replace it with the +relational assertion it actually means (`toBeLessThan(100)`), and leave a one-line comment saying which +property it holds. Where a test pins an exact worked example from a spec, keep it exact. + +- [ ] **Step 3: Keep the §12 worked example honest** + +`packages/core/test/score.test.ts` opens with a case named after design §12 that asserts the cap and the site +penalty together. Its route arithmetic changes; the cap at 79 and the site penalty in absolute points do not. +Update the route numbers, and add an assertion that `scoreModel.sitePenalty` is still the sum of absolute +deductions — that is the field the spec deliberately left alone, and nothing else in the suite pins it. + +- [ ] **Step 4: Run all three suites** + +```bash +(cd packages/core && ../../node_modules/.bin/vitest run) +(cd packages/cli && ../../node_modules/.bin/vitest run) +(cd packages/vite && ../../node_modules/.bin/vitest run) +``` + +Expected: all pass. + +- [ ] **Step 5: Rebuild core and typecheck every package** + +```bash +(cd packages/core && ../../node_modules/.bin/tsup) +for p in core cli vite; do (cd packages/$p && ../../node_modules/.bin/tsc --noEmit); done +``` + +Expected: clean. (`packages/mcp` has no `tsconfig.json`; skip it.) This per-package check is not ceremony — +on an earlier branch a cross-package break hid in a fixture that never named the changed type. + +- [ ] **Step 6: Commit** + +```bash +git add packages/core/test packages/cli/test packages/vite/test +git commit -m "test: re-baseline score expectations against the proportional model" +``` + +--- + +## Task 4: Documentation and changeset + +**Files:** + +- Modify: `docs/src/content/docs/guides/(reporting)/reporters.md` and + `docs/src/content/docs/ja/guides/(reporting)/reporters.md` +- Create: `.changeset/score-proportionality.md` + +**Interfaces:** + +- Consumes: the shipped behaviour from Tasks 1–3. Nothing is produced. + +- [ ] **Step 1: Find what the docs say about scores** + +```bash +grep -rn "score" "docs/src/content/docs/guides/(reporting)/reporters.md" +grep -rn "score" docs/src/content/docs/ja/guides/\(reporting\)/reporters.md +``` + +`routes[].score` is documented in both. Read the surrounding prose before editing — the page describes the +JSON report's shape, and only the description of what the number means changes. + +- [ ] **Step 2: Update both pages** + +State that a route's `score` is the share of the checks that ran on that route, weighted by severity, that +passed — not 100 minus a fixed deduction per finding. Keep it to the same length as the sentence it replaces. +Do not add a migration note to the docs; that belongs in the changeset. Do not name any other tool. + +- [ ] **Step 3: Check the rest of the docs for a stale claim** + +```bash +grep -rn "100 minus\|deduct\|15 points\|5 points\|1 point" docs/src/content/docs --include="*.md*" +``` + +Any prose describing the old subtraction is now wrong. Update what you find, in both languages. If nothing +matches, say so in the report rather than silently skipping the step. + +- [ ] **Step 4: Write the changeset** + +Create `.changeset/score-proportionality.md`: + +```md +--- +'@svelte-vitals/core': minor +'svelte-vitals': minor +'@svelte-vitals/vite': minor +--- + +Category scores now reflect **how much** is wrong, not merely whether anything is. + +A key — a route or a source file — used to start at 100 and lose a fixed number of points per failing rule. +That capped what a category could express: `architecture` is eight `info` rules, so no amount of bad code +moved it below 92, and three more scopes bottomed out above 90. It also erased magnitude, because one +finding moves a mean of N keys by `1/N`: on a large project, one finding and several hundred displayed the +same score. + +A key now scores the share of what it was measured against that is intact, weighted by severity. Every +category can reach 0, and the score moves with the number of findings. + +**Every score changes, most of them downward, and by more than a point.** `seo` and `correctness` stay +within a point of their old values; `architecture`, `security` and `performance` move further, because their +scales were the most compressed. A `--min-health` gate calibrated against the old numbers will start +failing — recalibrate it against the new scale. `routes[].score` in the JSON report changes meaning the same +way. Stored baselines are unaffected, since they key on findings rather than scores. + +Unchanged: the site-wide penalty stays in absolute points, a `critical` still caps a category at 79, and a +displayed 100 still means no finding among the checks that ran. +``` + +- [ ] **Step 5: Verify the docs build inputs** + +```bash +node_modules/.bin/oxfmt --check . +(cd packages/cli && ../../node_modules/.bin/vitest run test/docs-links.test.ts test/rules-index.test.mjs test/docs-embed.test.mjs) +``` + +Expected: clean and passing. If `oxfmt` reports a diff, run `node_modules/.bin/oxfmt --write .` and re-check. + +- [ ] **Step 6: Full verification** + +```bash +for p in core cli vite; do (cd packages/$p && ../../node_modules/.bin/vitest run); done +(cd packages/core && ../../node_modules/.bin/tsup) +for p in core cli vite; do (cd packages/$p && ../../node_modules/.bin/tsc --noEmit); done +node_modules/.bin/oxlint . && node_modules/.bin/oxfmt --check . +``` + +Expected: all green. + +- [ ] **Step 7: Commit** + +```bash +git add docs .changeset +git commit -m "docs: describe the proportional score model" +``` + +--- + +## Notes for whoever runs this + +- A full-workspace `pnpm` command fails in this sandbox for a known, pre-existing reason (the `docs` + package's dependencies). Use `--filter` or the per-package binaries shown above. Do not try to fix it. +- The spec records two things as deliberately **not** solved, so do not implement them here: + `routes[].categories[].score` in the JSON report, and severity recalibration of thin scopes. +- The spec accepts one tolerance: a multi-key mean whose true value is an integer may display one point low + (four `performance` component keys failing `{info}`, `{warning}`, `{all five}`, `{three infos}` display 49 + for a true 50). This is bounded at one point and is not a bug to fix. diff --git a/docs/superpowers/specs/2026-08-04-score-proportionality-design.md b/docs/superpowers/specs/2026-08-04-score-proportionality-design.md new file mode 100644 index 000000000..5c9f28f8c --- /dev/null +++ b/docs/superpowers/specs/2026-08-04-score-proportionality-design.md @@ -0,0 +1,328 @@ +# Score proportionality: a category score must move with how much is wrong — design + +**Date:** 2026-08-04 +**Status:** approved +**Origin:** the first follow-up recorded by `2026-07-31-score-honesty-design.md`, which raised its own +priority: "Trading '100 is a lie' for '99 says nothing' is the right trade only if the second half gets +fixed." + +## The problem + +Two symptoms, measured rather than supposed. They share one cause. + +**A category cannot reach most of its own scale.** A key loses `DEDUCTION[severity]` per failing rule, and +is only ever touched by rules of its own scope — component rules key on a file, route rules on a route — so +the lowest score it can reach is fixed by that scope's rule inventory: + +| category / scope | rules | max deduction on one key | lowest reachable | +| ------------------------ | --------------------------- | ------------------------ | ---------------- | +| architecture / component | 8 info | 8 | **92** | +| seo / component | 1 warning | 5 | **95** | +| performance / component | 4 info, 1 warning | 9 | **91** | +| performance / route | 3 info, 5 warning | 28 | 72 | +| security / component | 4 warning, 1 crit | 35 | 65 | +| correctness / component | 1 info, 10 warning, 3 crit | 96 | 4 | +| seo / route | 10 info, 14 warning, 2 crit | 110 | 0 | + +Architecture is eight `info` rules and nothing else, so an architecture score is a nine-value scale +presented as a hundred-value one. No amount of bad code moves it below 92, and the rule shipped +immediately before this spec — `architecture/doc-link-target` — can move a key by exactly one point. Three +more scopes sit above 90, so this is not one miscalibrated category: it is what the model does wherever a +scope's rules are few or cheap. + +**The average erases magnitude.** One `info` finding moves a mean of N keys by `1/N`. At the scale the +field report measured (585 keys), 1 finding and 276 findings both display **99**. + +**The cause is the same in both.** A per-key penalty of one point is too small for the mean to register, +and too small for the scale to be used. Fixing either symptom alone fixes neither: raising the deductions +without changing the model still leaves architecture bounded, and un-bounding architecture without making +each affected key cost more still leaves the mean pinned near 100. + +## What must remain true + +The previous spec established an invariant this one may not break: + +> A displayed 100 means the deduction was exactly zero. + +And severity must still order findings: within a category and scope, a `critical` must cost more than a +`warning`, which must cost more than an `info`. + +## Design + +### The model: a key scores the share of its scope that is intact + +``` +keyScore = inventoryWeight === 0 ? 100 : clamp(100 − (100 × failedWeight) / inventoryWeight) +``` + +- **`failedWeight`** — over the distinct rule ids that produced a penalized result on this key, the sum of + `DEDUCTION[effective severity]`, taking the maximum among duplicates. This is today's numerator + unchanged; only what it is divided by is new. +- **`inventoryWeight`** — `max(observedInventory, failedWeight)`, where `observedInventory` is the sum of + `DEDUCTION[severity]` over the selected rules whose **(category, scope) pair** is among the pairs + observed on this key. + +Everything downstream is unchanged: key scores are averaged, `sitePenalty` is subtracted, the cap is +applied, the result is floored. + +The reading is "of what this key was measured against, how much is intact" — the same shape as the +audit-weighted category score users already know from browser performance tooling. + +**Three details in that formula are load-bearing, and two of them are arithmetic.** + +**The pair, not the scope alone.** `computeScore` takes results, not a category: `scoresByCategory` buckets +first and calls it per category, but three call sites pass a multi-category set — `routes[].score` in the +JSON report, the console's "By route" tree, and the Vite plugin's overall score. Partitioning by scope alone +leaves those undefined. Summing over observed `(category, scope)` pairs defines them and reduces to the +single-category rule exactly when the input is single-category, so the two paths cannot disagree. A route +carrying one failing `seo` `warning` alongside passing `performance` route rules scores `100 − 500/138 = +96.38` against the union, where the `seo` bucket alone gives `95.45`; both are defined, and which applies +depends only on what was handed in. + +**The evaluation order.** `100 × (1 − f / i)` loses a full displayed point on values that are exactly +integral: with `f = 88, i = 110` it yields `19.999999999999996`, which floors to **19** for a true **20** +(and `f = 99` gives `9.999999999999998` → **9** for **10**; both are reachable on one `seo` route). Written +as `100 − (100 × f) / i`, `100 × f` and `i` are exact integers and an integer quotient of exact integers is +exactly representable — the same argument `score.ts` already makes for the route mean, whose comment must be +updated because this model breaks its stated premise that every key score is an integer. + +**`max(observedInventory, failedWeight)` and the zero guard.** Normally `observedInventory ≥ failedWeight`, +since every failing rule sits in its own pair's inventory, and the `max` is inert. It is there for the two +cases where that does not hold, both of which would otherwise divide by zero or produce `NaN` — +`clamp(NaN)` is `NaN`, which would propagate into the category score, into Health, and out as +`"score": null`: + +- `treatDynamicAs: 'warn'` promotes a **result's** severity without changing its rule's, so `failedWeight` + can exceed the inventory. The key scores 0, which is the honest reading. +- A result whose rule id is absent from the inventory — reachable through the `rules?:` escape hatch below, + or a set of results scored against a configuration that turned its rule `off`. It observes no pair, so it + contributes nothing to `observedInventory`; the `max` keeps it from being divided by zero and from + displaying 100 with a finding present, which would break the invariant this spec inherits. +- An `overrides` entry raising a rule's severity for a glob. `selectRules` and `buildInventory` read only + top-level `config.rules`, so the denominator keeps the rule's base severity while the result carries the + raised one. Measured on the eight-`info` architecture pair: promoting one rule's result to `critical` this + way scores the key **0** (`failedWeight` 15 against an unmoved inventory of 8), where making the same + promotion at top level — which raises the inventory too, to 22 — displays **31** (raw 31.81…). + +The zero guard covers the remaining case: a key with no penalized results and no observed pair scores 100, +matching today's seed. + +### Why the denominator is the scope's inventory and not what applied + +Three candidates were considered. The measurements decide between them. + +**Rejected: the rules that actually applied to this key.** This is the faithful "not applicable" semantics, +and it collapses. Measuring the `applies` condition of each architecture rule: + +| rule | applies when | +| ------------------------ | ------------------------------------ | +| `component-size` | `loc > 0` — every parsable component | +| `prop-count` | `propCount > 0` | +| `doc-link-target` | a declared-prefix link exists — rare | +| `route-component-import` | a route entry is imported — rare | +| the four directory rules | a declaration matches — L3 | + +So an ordinary component has **one or two** applicable architecture rules. A component with no props that +exceeds the size threshold would have `failedWeight === inventoryWeight` and score **0** — a file a few +lines over a threshold, scored as total failure. Worse, severity would invert across categories: an +`info` at denominator 1 costs 100 points while a `critical` at denominator 35 costs 43. + +Flooring the denominator does not rescue it, because the floor and the fix trade against each other +directly. At 300 keys with 10 affected, a per-key score of 0 yields a category mean of 96.7 and a per-key +score of 80 yields 99.3. **The larger the floor, the less of the fix survives**, and the constant has no +principle behind it. + +**Rejected: the category's whole inventory, unpartitioned.** `performance` and `seo` each carry rules of +more than one scope. A component key measured against route-scoped rules it can never trigger keeps a +guaranteed share of the denominator intact — the ceiling, reintroduced one level down. + +**Chosen: the inventory of the `(category, scope)` pairs observed on the key.** Partitioning removes the +objection to the whole-inventory form while keeping its stability. The denominator does not depend on which +checks happened to have something to say about this particular file, so no key collapses to a denominator of +one. + +### The unevenness this accepts, stated rather than smoothed + +Because the denominator is the scope's rule count, a scope with few rules charges more per finding: + +Key scores are **not** rounded — they are averaged, and only the category score is floored. The column +below is the exact key score, so a single-key category displays its floor: + +| key | inventory | one finding | displays | today | +| ----------------------------------- | --------- | ----------- | -------- | ----- | +| seo / route (`critical`) | 110 | 86.36 | 86 | 85 | +| correctness / component (`warning`) | 96 | 94.79 | 94 | 95 | +| security / component (`critical`) | 35 | 57.14 | 57 | 85 | +| performance / route (`warning`) | 28 | 82.14 | 82 | 95 | +| architecture / component (`info`) | 8 | 87.5 | 87 | 99 | +| performance / component (`warning`) | 9 | 44.44 | **44** | 95 | + +A `performance` `warning` therefore costs more than a `security` `critical` (44 against 57). Both scopes hold +five rules, but `security`'s carry four times the weight, so the same finding is a larger share of the +thinner denominator. Severity still orders findings **within** a category and scope, which is the guarantee +this spec keeps; it does not order them across categories, which the previous model did and this one does +not. + +This is accepted rather than corrected. The alternative — a floor under the denominator — buys a smoother +table at the cost of the fix itself, on an arbitrary constant. The unevenness also self-corrects: every rule +added to a thin scope widens its denominator. + +Note what does **not** move. `seo` and `correctness`, the two categories with the largest inventories and +the ones users read most often, land within a point of today. The change concentrates where the scale was +most compressed, which is the intended effect and not a side effect. + +### `sitePenalty` stays absolute + +Project-scoped rules keep subtracting `DEDUCTION[severity]` points directly from the category average. + +Neither symptom applies to them: a site-wide finding is not divided by the number of keys, so nothing +erases its magnitude, and the site penalty was never bounded by a per-key floor. Normalising it would move +scores for a reason this spec is not about — a single site-wide `warning` in `seo` would go from 5 points to 31. + +The consequence is a model that mixes scales: a key's deficit is a share of its scope's inventory, a site +penalty is points. It is bounded (the largest site penalty in the codebase is `seo`'s 16 points) and it is +recorded here rather than discovered later. + +### Everything else is unchanged + +`CRITICAL_CAP` at 79 with its decision on the raw value; `Math.floor` at both stages; `computeHealth` +averaging unrounded category scores in deficit space; "present" categories only; the empty case scoring 100. + +The invariant survives by construction: `failedWeight` is zero exactly when no penalized finding carries the +key, and only then does every key score 100 — the zero guard returns 100 precisely in the case where +`failedWeight` must also be zero, and the `max` keeps a penalized result whose rule is unknown from reaching +the guard. + +One premise in `score.ts` does **not** survive. The comment at the route mean argues that no epsilon is +needed there because "every route score is an integer, so `sum / length` is a division of exact integers, +and whenever the true quotient IS an integer it is exactly representable". Key scores are no longer +integers, and the conclusion the comment draws no longer follows from the reason it gives. + +**The clean case is not what breaks.** A clean key scores exactly `100`, a sum of exact `100`s is exact for +any key count this tool will ever see, and the quotient is exactly `100`. That guarantee survives the model +change untouched, and a test asserting it would pass under any arithmetic — it pins nothing. + +**What breaks is an exactly-integral mean of unclean keys.** Two `performance` component keys against an +inventory of 9, one failing three `info` rules and one failing a `warning` and an `info`, have a true +category score of exactly **50**. Computed as a mean of key scores it comes out `49.99999999999999` and +displays **49**. + +**So the route mean moves into deficit space**, as `computeHealth` already does: +`rawRouteAverage = 100 − (Σ keyDeficit) / N`, where `keyDeficit` is `(100 × failedWeight) / inventoryWeight`. +That fixture then displays 50. The comment must be rewritten to state this reason rather than the one it +states today — a false premise left in a comment about floating point is how the previous spec's arithmetic +bugs survived review the first time, and the first draft of this paragraph made exactly that mistake. + +**The residual, stated because the single-key case was not allowed to pass silently.** Deficit space narrows +this class; it does not close it. Four `performance` component keys failing one `info`, one `warning`, all +five rules, and three `info`s have a true mean of exactly 50 and display **49** in deficit space too. The +accepted tolerance is therefore: exact for a clean project and exact for a single key, but a multi-key mean +whose true value is an integer may display one point low. It is bounded at one point and it is the same +tolerance `computeHealth` already carries — but this spec blocked the single-key instance of the identical +error as "a full displayed point lost", so leaving the multi-key instance unnamed would be the double +standard. + +### Wiring: no signature cascade + +`computeScore` needs a rule inventory, which it has never had. It does not need a new argument. + +`ScoreOptions` gains `rules?: readonly Rule[]`, defaulting to `selectRules(allRules, config)` computed +inside `computeScore`. A supplied list is not taken as given either: `computeScore` runs it through +`selectRules(config)` itself, so an injected rule that config turns `off` is dropped from the inventory and +from `pairOf` alike, rather than reaching one but not the other. Nothing under `packages/core/src/rules/` +imports anything under +`packages/core/src/scoring/`, so the import direction is free — verified before choosing this shape. Every +existing call site — three reporters in `core`, three in `cli`, one in `vite` — is untouched, and the +optional parameter remains for tests and for scoring against a rule set that is not the registry. The Vite +plugin already bundles `allRules` through `analyze.ts`, so nothing gains weight. + +This is deliberately unlike `2026-08-03-json-rule-evidence`, which threaded a rule-id list through +`AnalyzeResult` because the CLI narrows rules by `--category` _after_ selection and the reporter needed the +narrowed set. Here the narrowed set would be wrong: `--category seo` must not shrink `seo`'s own inventory, +or a filtered run would score differently from a full one on identical input. The inventory is a property of +the configuration, not of the run. + +**Severity for the denominator** is `settingSeverity(config.rules[id]) ?? rule.severity` — the same +top-level resolution `selectRules` uses. `overrides` narrows severity per glob and `selectRules` does not +read it, so a rule disabled only inside an override stays in the inventory. + +## What this buys, quantified + +The resolution improves by the ratio of the new per-key deficit to the old one, and no further. In +`architecture`, one `info` used to cost a key 1 point and now costs 12.5, so at N keys it takes +`⌊N/12.5⌋ + 1` findings to move the displayed score by one instead of `N + 1`. The `+ 1` is not a rounding +convenience: a mean deficit of exactly 1 still floors to 99, so at N = 500 forty findings display 99 and +forty-one are needed. At the field report's 585 keys that is 47 findings rather than 586 — the reported case +(276) now reads **94** against **99** for one finding, but **1 through 46 findings all still display 99.** + +That flat band is the honest limit of this change: it makes magnitude visible at the scale the complaint was +filed at, and it does not make every increment visible. Stating the factor here is what keeps the same +complaint from being re-filed at 20 findings as though nothing had been done. + +## Migration + +Every score moves, most of them down, and by much more than the previous spec's one point. The changeset +must say so plainly, along with the three consequences: + +- a `--min-health` gate calibrated against today's numbers will start failing, and the fix is to + recalibrate against the new scale, not to work around it; +- **`routes[].score` in the JSON report changes meaning**, from "100 minus this route's deductions" to "the + share of everything measured on this route that is intact". It is a published field, documented in + `docs/src/content/docs/guides/(reporting)/reporters.md` (and its Japanese counterpart), whose description + of the field must be updated alongside; +- a stored baseline is unaffected — baselines key on findings, not scores. + +## Testing + +1. **Magnitude is visible.** At 585 keys, one affected key and 276 affected keys must display **different** + scores. Under the current model both display 99, so this test must fail before the change. This is the + regression test for the reported symptom. +2. **The scale is reachable.** An architecture key failing every architecture rule scores 0, not 92. A test + asserting merely "below 92" would pass on a model that only widened the deductions. +3. **The invariant holds from both sides.** No penalized finding → 100. A single `info` among many passes → + never 100. +4. **The denominator is partitioned by scope.** A `performance` component key must not count + `performance`'s route-scoped rules. Assert the exact score of a component key with one failing + component-scoped rule; computed against the unpartitioned inventory it is measurably higher, so this + test is what holds the partition in place. Take the expected value from the formula, not from this + document's tables — those are illustrative and go stale on the next rule added. +5. **A multi-category key sums the observed pairs.** Score a single route carrying one failing `seo` + `warning` and a passing `performance` route rule, through `computeScore` directly rather than through + `scoresByCategory`. Against the `seo` bucket alone the answer is higher, so this is the test that holds + the pair definition — and it is the path `routes[].score`, the console route tree and the Vite plugin + take. Without it the multi-category call sites are unpinned. +6. **Severity still orders within a scope.** In one category and scope, a `critical` scores lower than a + `warning`, which scores lower than an `info`. Assert the ordering, not the absolute values, so rule + additions do not churn the test. +7. **The inventory follows configuration, not the tree.** Turning a rule `off` removes it from the + denominator, so the remaining findings cost more. This pins the inventory to `selectRules` rather than to + which rules happened to produce results. +8. **`--category` does not change the scored category's score.** Score one category's results with the full + rule set and with a rule set narrowed to that category; the category score must be identical. Compare + only that category — a narrowed rule set leaves the other categories with no inventory, which is the + unknown-rule path, not the invariance being asserted. +9. **No input produces `NaN` or a 100 that hides a finding.** A penalized result whose rule is absent from + the injected inventory scores its key 0, not `NaN` and not 100. A key with no results and no inventory + scores 100. Assert `Number.isFinite` on the category score in both. +10. **Integral scores survive the arithmetic, in both places it can go wrong.** A single key with + `failedWeight` 88 against inventory 110 displays **20** — written as `100 × (1 − f / i)` it yields + `19.999999999999996` and displays 19, so this holds the evaluation order. And two `performance` + component keys against an inventory of 9, failing `f = 3` and `f = 6`, display **50** — as a mean of key + scores that is `49.99999999999999` and displays 49, so this holds the deficit-space mean. Do **not** + assert that a clean project displays 100 as the test for the mean: it is exactly 100 under either + arithmetic, so it would pass on the implementation this test exists to reject. +11. **Unchanged edges.** No results → 100. All passes → 100. A `critical` still caps a category at 79. + `sitePenalty` still subtracts absolute points — a site-wide `warning` in `seo` costs 5, not 31. + +## Deliberately not solved + +- **`routes[].categories[].score` in the JSON report.** Still the third follow-up from the previous spec, + and this change sharpens the need: a reader who wants to know why a category moved now has to reconstruct + a ratio per key rather than a subtraction. Recorded, not closed. +- **Making the directory-rule family verifiable from its output.** Unchanged and unaffected. +- **Severity recalibration.** The unevenness above has a second cause the model cannot reach: `performance` + having five component-scoped rules is a property of the rule set, not of the scoring. Widening thin + scopes is rule work, tracked separately. +- **Health's cross-category weighting.** `computeHealth` averages category scores with configurable + weights. Whether those defaults are still right once categories use their full range is a question this + spec raises and does not answer. diff --git a/packages/core/src/scoring/inventory.ts b/packages/core/src/scoring/inventory.ts new file mode 100644 index 000000000..2dbbf5ee3 --- /dev/null +++ b/packages/core/src/scoring/inventory.ts @@ -0,0 +1,46 @@ +import type { Category, Config, Scope, Severity } from '../types.js'; +import type { Rule } from '../rule.js'; +import { selectRules, settingSeverity } from '../config-apply.js'; +import { allRules } from '../rules/index.js'; + +export const DEDUCTION: Record = { critical: 15, warning: 5, info: 1 }; + +export type PairKey = `${Category}::${Scope}`; + +export function pairKey(category: Category, scope: Scope): PairKey { + return `${category}::${scope}`; +} + +/** A rule's severity as configured, or undefined if the config turns it off. */ +function severityOf(rule: Rule, config: Config): Severity | undefined { + const setting = settingSeverity(config.rules[rule.id]); + if (setting === 'off') return undefined; + return setting ?? rule.severity; +} + +/** + * Total severity weight per `(category, scope)` pair — the denominator a key of that pair is measured + * against. Defaults to the selected registry so `computeScore` needs no new argument; the parameter exists + * for tests and for scoring against a rule set that is not the registry. + */ +export function buildInventory( + config: Config, + rules: readonly Rule[] = selectRules(allRules, config) +): Map { + const out = new Map(); + for (const rule of rules) { + // An 'off' rule contributes nothing to the denominator it would otherwise be measured + // against — checked here rather than trusted to `selectRules`, since a rule list passed + // directly (as tests do) bypasses it. + const severity = severityOf(rule, config); + if (severity === undefined) continue; + const key = pairKey(rule.category, rule.scope); + out.set(key, (out.get(key) ?? 0) + DEDUCTION[severity]); + } + return out; +} + +/** Rule id to its pair, so a result can be attributed to the inventory entry it was measured against. */ +export function ruleScopes(rules: readonly Rule[]): Map { + return new Map(rules.map((r) => [r.id, pairKey(r.category, r.scope)])); +} diff --git a/packages/core/src/scoring/score.ts b/packages/core/src/scoring/score.ts index 82a3b22c8..b39ccd3a7 100644 --- a/packages/core/src/scoring/score.ts +++ b/packages/core/src/scoring/score.ts @@ -1,8 +1,11 @@ -import type { Category, Config, Result, Severity } from '../types.js'; +import type { Category, Config, Result } from '../types.js'; +import type { Rule } from '../rule.js'; import { isPenalized } from '../rule.js'; import { effectiveSeverity } from '../summary.js'; +import { selectRules } from '../config-apply.js'; +import { allRules } from '../rules/index.js'; +import { buildInventory, ruleScopes, DEDUCTION, type PairKey } from './inventory.js'; -const DEDUCTION: Record = { critical: 15, warning: 5, info: 1 }; const CRITICAL_CAP = 79; export interface ScoreModel { @@ -26,6 +29,8 @@ export interface ScoreResult { export interface ScoreOptions { applyCriticalCap?: boolean; + /** The rules that ran. Defaults to the selected registry; supplied by tests and custom rule sets. */ + rules?: readonly Rule[]; } function clamp(n: number): number { @@ -37,38 +42,55 @@ export function computeScore(results: Result[], config: Config, options: ScoreOp const routeResults = results.filter((r) => r.route !== undefined); const projectResults = results.filter((r) => r.route === undefined); - // Seed every route at 100 so passing routes count toward the average. - const routeScores = new Map(); - for (const r of routeResults) if (!routeScores.has(r.route as string)) routeScores.set(r.route as string, 100); + // `selectRules` applied here, not just inside `buildInventory`, so `pairOf` and the inventory see + // the same filtered list — an injected rule that config turns `off` must vanish from both, not map + // to a pair in one and contribute nothing in the other. + const rules = selectRules([...(options.rules ?? allRules)], config); + const inventory = buildInventory(config, rules); + const pairOf = ruleScopes(rules); let anyCritical = false; - // One deduction per (route, rule id): take the max deduction among duplicates, - // then sum a route's per-rule deductions. Keyed by a nested map so route paths - // never need to be parsed back out of a composite string key. - const routeRuleMax = new Map>(); + // Per key: the pairs it was measured against, and the weight that failed. One deduction per + // (key, rule id) — the max among duplicates — exactly as before; only the divisor is new. + const observed = new Map>(); + const ruleMax = new Map>(); for (const r of routeResults) { + const key = r.route as string; + if (!observed.has(key)) observed.set(key, new Set()); + const pair = pairOf.get(r.id); + if (pair !== undefined) observed.get(key)!.add(pair); if (!isPenalized(r.detection, config.treatDynamicAs)) continue; const sev = effectiveSeverity(r, config); if (sev === 'critical') anyCritical = true; - const route = r.route as string; - let perRule = routeRuleMax.get(route); - if (!perRule) routeRuleMax.set(route, (perRule = new Map())); + let perRule = ruleMax.get(key); + if (!perRule) ruleMax.set(key, (perRule = new Map())); const prev = perRule.get(r.id) ?? 0; if (DEDUCTION[sev] > prev) perRule.set(r.id, DEDUCTION[sev]); } - for (const [route, perRule] of routeRuleMax) { - let deduction = 0; - for (const d of perRule.values()) deduction += d; - routeScores.set(route, (routeScores.get(route) as number) - deduction); + + // Deficit space, as `computeHealth` already works: a mean of key scores computes + // 49.99999999999999 for a true 50 on two keys of deficit 300/9 and 600/9. + let totalDeficit = 0; + for (const [key, pairs] of observed) { + let failed = 0; + for (const d of ruleMax.get(key)?.values() ?? []) failed += d; + let inventoryWeight = 0; + // `pairOf` and `inventory` are built from the same filtered `rules`, so every pair reachable + // through `pairOf` also has an inventory entry; the `?? 0` is a degrade-to-0-not-NaN fallback + // against a future divergence between the two, not a path this file's own inputs can reach. + for (const p of pairs) inventoryWeight += inventory.get(p) ?? 0; + // `max` keeps `failed` from exceeding its own denominator, so a penalized finding can never still + // score 100: the two ways that would happen are `treatDynamicAs: 'warn'` promoting a result's + // severity above its rule's, and a result whose rule is absent from the inventory. + inventoryWeight = Math.max(inventoryWeight, failed); + // `100 - (100 * f) / i`, never `100 * (1 - f / i)`: the latter gives 19.999999999999996 for + // f = 88, i = 110 and displays 19 for a true 20. + totalDeficit += inventoryWeight === 0 ? 0 : (100 * failed) / inventoryWeight; } - const scores = [...routeScores.values()].map(clamp); - const rawRouteAverage = scores.length ? scores.reduce((a, b) => a + b, 0) / scores.length : 100; - // No such treatment needed here (contrast computeHealth below, which works in deficit space and caps - // at 99 rather than trusting the arithmetic): every route score is an integer, so `sum / length` is a - // division of exact integers, and whenever the true quotient IS an integer it is exactly representable - // — this mean can never come out as 99.99999999999999. + const keyCount = observed.size; + const rawRouteAverage = keyCount === 0 ? 100 : 100 - totalDeficit / keyCount; const routeAverage = Math.floor(rawRouteAverage); // One deduction per project rule id: take the max deduction among duplicates. diff --git a/packages/core/test/health.test.ts b/packages/core/test/health.test.ts index 32ed29d63..0dc89ecb1 100644 --- a/packages/core/test/health.test.ts +++ b/packages/core/test/health.test.ts @@ -93,22 +93,54 @@ const cat = (category: Category, keys: number, findings: number): Result[] => })); describe('computeHealth — one rounding, at the boundary', () => { - /** `keys` keys in `category`, the first `findings` of them carrying one finding of `severity`. */ - const at = (category: Category, keys: number, findings: number, severity: 'info' | 'warning'): Result[] => - cat(category, keys, 0).map((r, i) => - i < findings ? { ...r, severity, detection: { presence: 'none' as const, value: 'absent' as const } } : r - ); + /** + * `keys` keys on a real registry rule id (not the ghost `${category}/x`), the first `findings` + * penalized at `severity`. Because the id has a real (category, scope) inventory, a failing key's + * deficit runs through the general `100 * failed / max(inventory, failed)` path instead of the + * zero-inventory edge case, where a ghost id always wipes a penalized key to a 100% deficit + * regardless of severity (see score.test.ts "scores 0, not NaN, for a penalized result whose rule is + * not in the inventory"). + */ + const realKeys = ( + id: string, + category: Category, + keys: number, + findings: number, + severity: 'info' | 'warning' | 'critical' + ): Result[] => + Array.from({ length: keys }, (_, i) => ({ + id, + category, + severity, + detection: + i < findings + ? { presence: 'none' as const, value: 'absent' as const } + : { presence: 'own' as const, value: 'static' as const }, + route: `${category}-k${i}`, + message: 'm', + recommendation: 'r' + })); it('floors the mean of the unrounded category scores, not of the displayed ones', () => { - // Raw category scores [99.9, 99.9, 99.9, 99.9, 97.9] → mean 99.5 → 99. - // Averaging the floored scores [99, 99, 99, 99, 97] gives 98.6 → 98: a two-point move, which is - // what this asserts against. 100 info findings over 1000 keys = 99.9; 420 warnings = 97.9. + // One info-severity (deduction 1) finding costs 100/I per failing key against a real inventory I. + // Choosing exactly F = I failing keys out of 1000 makes the category's total deficit exactly + // I * (100/I) = 100, so raw = 100 - 100/1000 = 99.9 — independent of which real (category, scope) + // inventory I is used: + // seo::route (I=110): 110 of 1000 keys fail -> 99.9 + // performance::route (I=28): 28 of 1000 keys fail -> 99.9 + // correctness::component (I=96): 96 of 1000 fail -> 99.9 + // security::component (I=35): 35 of 1000 fail -> 99.9 + // architecture::component (I=8) uses 21*I = 168 failing keys instead: each costs 100*1/8 = 12.5, + // so 168 * 12.5 / 1000 = 2.1 deficit -> raw = 97.9. + // Raw category scores [99.9, 99.9, 99.9, 99.9, 97.9] -> mean 99.5 -> floor 99. + // Averaging the floored scores [99, 99, 99, 99, 97] first gives 98.6 -> floor 98, which is what + // this test asserts against. const results = [ - ...at('seo', 1000, 100, 'info'), - ...at('performance', 1000, 100, 'info'), - ...at('correctness', 1000, 100, 'info'), - ...at('security', 1000, 100, 'info'), - ...at('architecture', 1000, 420, 'warning') + ...realKeys('seo/title-presence', 'seo', 1000, 110, 'info'), + ...realKeys('performance/image-loading-hint', 'performance', 1000, 28, 'info'), + ...realKeys('correctness/unmutated-state', 'correctness', 1000, 96, 'info'), + ...realKeys('security/raw-html', 'security', 1000, 35, 'info'), + ...realKeys('architecture/component-size', 'architecture', 1000, 168, 'info') ]; expect(computeHealth(results, CONFIG).health).toBe(99); }); @@ -145,10 +177,12 @@ describe('computeHealth — one rounding, at the boundary', () => { expect(computeHealth([], CONFIG).health).toBe(100); // A zero-weight category is present (its rule ran) and can hold a penalized finding — here severe // enough to cap its own score at 79 — without moving Health at all: the invariant is bounded to - // positively weighted categories, not merely present ones. - const penalizedSecurity: Result[] = cat('security', 10, 10).map((r, i) => - i === 0 ? { ...r, severity: 'critical' as const } : r - ); + // positively weighted categories, not merely present ones. One critical (deduction 15) amid 199 + // clean keys on the real `security/raw-html` id (security::component inventory 35) costs + // 100*15/35 = 42.857... against its one key, so raw = 100 - 42.857.../200 = 99.7857...; any + // critical present caps a raw score above 79 down to exactly 79 regardless of how small the + // underlying deficit is. + const penalizedSecurity: Result[] = realKeys('security/raw-html', 'security', 200, 1, 'critical'); const zeroWeighted = computeHealth( [...cat('seo', 10, 0), ...penalizedSecurity], defineConfig({ weights: { seo: 1, security: 0 } }) @@ -158,9 +192,11 @@ describe('computeHealth — one rounding, at the boundary', () => { }); it('treats a weight small enough to underflow an epsilon as still not clean (regression)', () => { - // seo carries a single `info` finding on its only key, so its rawScore is exactly 99. Weighted at - // 1e-12 alongside a clean weight-1 category, the true quotient is 99.999999999999 — below 100, but - // only by ~1e-12. The old `Math.floor(weighted / total + 1e-9)` guard rounded that up to 100, + // seo carries a single `info` finding on its only key; `${category}/x` has no inventory (see the + // test above), so the key takes a full 100% deficit and seo's rawScore is 0, not merely reduced. + // Weighted at 1e-12 alongside a clean weight-1 category, the true quotient is 99.9999999999 — below + // 100, but only by ~1e-10 (the full 100-point deficit times the negligible weight share). The old + // `Math.floor(weighted / total + 1e-9)` guard rounded that up to 100, // silently letting a real finding hide behind a tiny weight and pass `--min-health 100`. Working in // deficit space has no such blind spot: the deficit here is strictly positive (not the exact-zero // case), so it is floored down, not tolerance-forgiven. Confirmed this fails (returns 100) against diff --git a/packages/core/test/inventory.test.ts b/packages/core/test/inventory.test.ts new file mode 100644 index 000000000..a494dc30d --- /dev/null +++ b/packages/core/test/inventory.test.ts @@ -0,0 +1,45 @@ +import { describe, it, expect } from 'vitest'; +import { buildInventory, pairKey, ruleScopes } from '../src/scoring/inventory.js'; +import { defineConfig } from '../src/types.js'; +import { allRules } from '../src/rules/index.js'; +import type { Rule } from '../src/rule.js'; + +const rule = (id: string, category: Rule['category'], scope: Rule['scope'], severity: Rule['severity']) => + ({ id, category, scope, severity, title: id, rationale: '', check: async () => [] }) as unknown as Rule; + +describe('buildInventory', () => { + it('sums DEDUCTION per (category, scope) pair', () => { + const rules = [ + rule('a/one', 'architecture', 'component', 'info'), + rule('a/two', 'architecture', 'component', 'warning'), + rule('p/one', 'performance', 'route', 'critical') + ]; + const inv = buildInventory(defineConfig({}), rules); + expect(inv.get(pairKey('architecture', 'component'))).toBe(6); + expect(inv.get(pairKey('performance', 'route'))).toBe(15); + expect(inv.get(pairKey('performance', 'component'))).toBeUndefined(); + }); + + it('drops a rule turned off and counts a rule whose severity is overridden', () => { + const rules = [ + rule('a/one', 'architecture', 'component', 'info'), + rule('a/two', 'architecture', 'component', 'info') + ]; + const config = defineConfig({ rules: { 'a/one': 'off', 'a/two': 'critical' } }); + const inv = buildInventory(config, rules); + expect(inv.get(pairKey('architecture', 'component'))).toBe(15); + }); + + it('defaults to the selected registry', () => { + // Eight architecture rules, all info, is what makes the old model bottom out at 92. + const inv = buildInventory(defineConfig({})); + const architecture = allRules.filter((r) => r.category === 'architecture'); + expect(inv.get(pairKey('architecture', 'component'))).toBe(architecture.length); + }); + + it('maps a rule id to its pair', () => { + const rules = [rule('a/one', 'architecture', 'component', 'info')]; + expect(ruleScopes(rules).get('a/one')).toBe(pairKey('architecture', 'component')); + expect(ruleScopes(rules).get('nope')).toBeUndefined(); + }); +}); diff --git a/packages/core/test/score.test.ts b/packages/core/test/score.test.ts index a03a0cf35..8f9822d39 100644 --- a/packages/core/test/score.test.ts +++ b/packages/core/test/score.test.ts @@ -1,6 +1,7 @@ // Scores are floored, not rounded (2026-07-31): a displayed 100 means the deduction was exactly zero. import { describe, it, expect } from 'vitest'; import { computeScore, defineConfig, scoresByCategory, type Result } from '../src/index.js'; +import type { Rule } from '../src/rule.js'; const pass = (id: string, route: string): Result => ({ id, @@ -24,22 +25,30 @@ describe('computeScore (§12 worked example)', () => { pass('seo/title-presence', '/b'), pass('seo/title-presence', '/c'), pass('seo/title-presence', '/d'), - // route /blog: critical + 2 warnings + 1 info => 100-15-5-5-1 = 74 + // route /blog: critical(15) + warning(5) + warning(5) + info(1) = 26 failed, against the real + // seo::route registry inventory (110) -> key score 100 - 2600/110 = 76.36... fail('seo/description-presence', '/blog', 'critical'), fail('seo/canonical-url', '/blog', 'warning'), fail('seo/og-image', '/blog', 'warning'), fail('seo/json-ld', '/blog', 'info'), - // project rule: robots.txt missing (warning) => site penalty 5 + // two distinct project rules, so sitePenalty below demonstrably sums (5 + 1) rather than + // echoing a single deduction — the one field the proportional rewrite left untouched. { id: 'seo/robots-txt', severity: 'warning', detection: { presence: 'none', value: 'absent' }, message: 'no robots' + }, + { + id: 'seo/sitemap-in-robots', + severity: 'info', + detection: { presence: 'none', value: 'absent' }, + message: 'no sitemap link' } ]; const { score, scoreModel } = computeScore(results, defineConfig({})); - expect(scoreModel.routeAverage).toBe(94); // (100*4 + 74)/5 = 94.8 -> floor 94 - expect(scoreModel.sitePenalty).toBe(5); + expect(scoreModel.routeAverage).toBe(95); // (100*4 + 76.36..)/5 = 95.27 -> floor 95 + expect(scoreModel.sitePenalty).toBe(6); // DEDUCTION.warning + DEDUCTION.info = 5 + 1 expect(scoreModel.criticalCap).toBe(79); expect(score).toBe(79); }); @@ -51,7 +60,8 @@ describe('computeScore (§12 worked example)', () => { }); it('reports criticalCap null when the cap does not actually lower the score', () => { - // /x: critical (15) + 5 warnings (25) => 100-40 = 60, already below the 79 cap. + // /x: critical(15) + 5 warnings(5 each) = 40 failed, against inventory 110 -> key score + // 100 - 4000/110 = 63.63, floored to 63 - well below the 79 cap. const results: Result[] = [ fail('seo/description-presence', '/x', 'critical'), fail('seo/canonical-url', '/x', 'warning'), @@ -61,7 +71,7 @@ describe('computeScore (§12 worked example)', () => { fail('seo/twitter-card', '/x', 'warning') ]; const { score, scoreModel } = computeScore(results, defineConfig({})); - expect(score).toBe(60); + expect(score).toBe(63); expect(scoreModel.criticalCap).toBeNull(); }); @@ -76,7 +86,8 @@ describe('computeScore (§12 worked example)', () => { } ]; expect(computeScore(results, defineConfig({})).score).toBe(79); // capped (default) - expect(computeScore(results, defineConfig({}), { applyCriticalCap: false }).score).toBe(85); // uncapped: 100-15 + // uncapped: failed 15 of inventory 110 -> 100 - 1500/110 = 86.36, floored 86 + expect(computeScore(results, defineConfig({}), { applyCriticalCap: false }).score).toBe(86); }); it('deducts once per (route, rule) even if a rule emits duplicate penalized results', () => { @@ -96,8 +107,9 @@ describe('computeScore (§12 worked example)', () => { message: 'b' } ]; - // one deduction per (route, rule), taking the max (critical = 15) -> 100-15 = 85, uncapped view - expect(computeScore(results, defineConfig({}), { applyCriticalCap: false }).score).toBe(85); + // one deduction per (route, rule), taking the max (critical = 15) -> failed 15 of inventory 110 + // -> 100 - 1500/110 = 86.36, floored 86, uncapped view + expect(computeScore(results, defineConfig({}), { applyCriticalCap: false }).score).toBe(86); }); it('deducts once per project rule even if duplicated', () => { @@ -203,7 +215,8 @@ describe('computeScore — a displayed 100 means zero deduction', () => { it('exposes the unrounded score alongside the floored one', () => { const r = computeScore(spread(585, 276), CONFIG); expect(r.score).toBe(99); - expect(r.rawScore).toBeCloseTo(99.528, 3); + // 276 keys fail (id in the real seo::route inventory, 110): 100 - (276*100/110)/585 = 99.571095571... + expect(r.rawScore).toBeCloseTo(99.571, 3); }); it('floors routeAverage too, keeping score = routeAverage - sitePenalty when neither cap nor clamp binds', () => { @@ -276,3 +289,204 @@ describe('computeScore — a displayed 100 means zero deduction', () => { expect(r.scoreModel.criticalCap).toBe(79); }); }); + +const r = (id: string, category: Rule['category'], scope: Rule['scope'], severity: Rule['severity']) => + ({ id, category, scope, severity, title: id, rationale: '', check: async () => [] }) as unknown as Rule; + +// Nine weight in one pair — the shape that makes the arithmetic below checkable by hand. +const PERF = [ + r('p/i1', 'performance', 'component', 'info'), + r('p/i2', 'performance', 'component', 'info'), + r('p/i3', 'performance', 'component', 'info'), + r('p/i4', 'performance', 'component', 'info'), + r('p/w1', 'performance', 'component', 'warning') +]; + +describe('computeScore — proportional model', () => { + const config = defineConfig({}); + + it('scores a key as the share of its pair that is intact', () => { + // failedWeight 5 of inventory 9 -> 100 - 500/9 = 44.44…, floored once at the category. + const results = [fail('p/w1', 'src/A.svelte', 'warning')]; + const { score } = computeScore(results, config, { rules: PERF }); + expect(score).toBe(44); + }); + + it('lets a key reach 0 when everything in its pair fails', () => { + const results = PERF.map((rule) => fail(rule.id, 'src/A.svelte', rule.severity as 'warning' | 'info')); + expect(computeScore(results, config, { rules: PERF, applyCriticalCap: false }).score).toBe(0); + }); + + it('distinguishes one affected key from many', () => { + // The reported symptom: under the old model both displayed 99. + const keys = Array.from({ length: 585 }, (_, i) => `src/${i}.svelte`); + const one = keys.map((k, i) => (i === 0 ? fail('p/i1', k, 'info') : pass('p/i1', k))); + const many = keys.map((k, i) => (i < 276 ? fail('p/i1', k, 'info') : pass('p/i1', k))); + const a = computeScore(one, config, { rules: PERF }).score; + const b = computeScore(many, config, { rules: PERF }).score; + expect(a).toBe(99); + expect(b).toBeLessThan(a); + }); + + it('sums the inventory over every pair observed on a key', () => { + // One seo route warning beside a passing performance route rule: 100 - 500/(5+5) = 50, + // where the seo pair alone would give 100 - 500/5 = 0. + const rules = [r('seo/x', 'seo', 'route', 'warning'), r('perf/y', 'performance', 'route', 'warning')]; + const results = [fail('seo/x', '/a', 'warning'), pass('perf/y', '/a')]; + expect(computeScore(results, defineConfig({}), { rules }).score).toBe(50); + }); + + it('keeps an integral score integral', () => { + // NOT load-bearing on its own: f = 88, i = 110 also displays 20 under both wrong evaluation + // orders (100 * (1 - f/i) and 100 * (f/i)) — the deficit-space mean's subtraction happens to + // cancel their fp error at this particular anchor. Kept only as a worked example alongside the + // fixture below, which is the one that actually discriminates. + const rules = [ + r('s/c1', 'seo', 'route', 'critical'), + r('s/c2', 'seo', 'route', 'critical'), + ...Array.from({ length: 14 }, (_, i) => r(`s/w${i}`, 'seo', 'route', 'warning')), + ...Array.from({ length: 10 }, (_, i) => r(`s/i${i}`, 'seo', 'route', 'info')) + ]; + // 2 criticals (30) + 11 warnings (55) + 3 infos (3) = 88, against an inventory of 110. + const results = [ + fail('s/c1', '/a', 'critical'), + fail('s/c2', '/a', 'critical'), + ...Array.from({ length: 11 }, (_, i) => fail(`s/w${i}`, '/a', 'warning')), + ...Array.from({ length: 3 }, (_, i) => fail(`s/i${i}`, '/a', 'info')) + ]; + expect(computeScore(results, defineConfig({}), { rules, applyCriticalCap: false }).score).toBe(20); + }); + + it('scores 44 for failedWeight 28 against inventory 50', () => { + // f = 28, i = 50: 100 - (100*28)/50 is exactly 44; both 100 * (1 - 28/50) and 100 * (28/50) + // land on 43.99999999999999, displaying 43. Unlike the f=88/i=110 case above, this fixture + // fails under either wrong evaluation order — delete this one, not that one, if one has to go. + const rules = [ + r('s/c1', 'seo', 'route', 'critical'), + ...Array.from({ length: 7 }, (_, i) => r(`s/w${i}`, 'seo', 'route', 'warning')) + ]; // inventory: 15 + 7*5 = 50 + const results = [ + fail('s/c1', '/a', 'critical'), + fail('s/w0', '/a', 'warning'), + fail('s/w1', '/a', 'warning'), + fail('s/w2', '/a', 'info'), + fail('s/w3', '/a', 'info'), + fail('s/w4', '/a', 'info') + ]; // failed: 15 + 5+5 + 1+1+1 = 28 + expect(computeScore(results, defineConfig({}), { rules, applyCriticalCap: false }).score).toBe(44); + }); + + it('scores 50 for a two-key mean of failed weight 2 and 9 against inventory 11', () => { + // Two keys sharing an inventory of 11 (11 info rules), failing weight 2 and 9: true mean is + // exactly 50. Either wrong evaluation order lands the mean at 49.99999999999999, displaying 49. + const rules = Array.from({ length: 11 }, (_, i) => r(`p/i${i}`, 'performance', 'component', 'info')); + const results = [ + ...Array.from({ length: 2 }, (_, i) => fail(`p/i${i}`, 'src/A.svelte', 'info')), + ...Array.from({ length: 9 }, (_, i) => fail(`p/i${i}`, 'src/B.svelte', 'info')) + ]; + expect(computeScore(results, defineConfig({}), { rules }).score).toBe(50); + }); + + it('keeps an integral mean integral across keys', () => { + // Two keys, deficits 300/9 and 600/9, true mean exactly 50. A mean of key scores gives + // 49.99999999999999 and displays 49. + const results = [ + fail('p/i1', 'src/A.svelte', 'info'), + fail('p/i2', 'src/A.svelte', 'info'), + fail('p/i3', 'src/A.svelte', 'info'), + fail('p/w1', 'src/B.svelte', 'warning'), + fail('p/i1', 'src/B.svelte', 'info') + ]; + expect(computeScore(results, config, { rules: PERF }).score).toBe(50); + }); + + it('scores 0, not NaN, for a penalized result whose rule is not in the inventory', () => { + const results = [fail('ghost/rule', 'src/A.svelte', 'warning')]; + const { score } = computeScore(results, config, { rules: PERF, applyCriticalCap: false }); + expect(Number.isFinite(score)).toBe(true); + expect(score).toBe(0); + }); + + it('scores 100 for a key whose only results come from rules outside the inventory', () => { + const results = [pass('ghost/rule', 'src/A.svelte')]; + expect(computeScore(results, config, { rules: PERF }).score).toBe(100); + }); + + it('narrowing the rule set to one category leaves that category unchanged', () => { + const mixed = [...PERF, r('seo/x', 'seo', 'route', 'warning')]; + const results = [fail('p/w1', 'src/A.svelte', 'warning'), pass('p/i1', 'src/A.svelte')]; + expect(computeScore(results, config, { rules: mixed }).score).toBe( + computeScore(results, config, { rules: PERF }).score + ); + }); + + it('keeps a category with two scopes from merging them', () => { + // A component key must not be measured against route-scoped rules it can never trigger. + // Merged, the inventory would be 5 + 45 and the key would score 90 instead of 0. + const rules = [ + r('p/comp', 'performance', 'component', 'warning'), + ...Array.from({ length: 9 }, (_, i) => r(`p/route${i}`, 'performance', 'route', 'warning')) + ]; + const results = [fail('p/comp', 'src/A.svelte', 'warning')]; + expect(computeScore(results, config, { rules, applyCriticalCap: false }).score).toBe(0); + }); + + it('scores 100 when nothing is penalized', () => { + const results = [pass('p/i1', 'src/A.svelte'), pass('p/w1', 'src/B.svelte')]; + expect(computeScore(results, config, { rules: PERF }).score).toBe(100); + }); + + it('orders severities within one pair', () => { + const one = (id: string, sev: 'critical' | 'warning' | 'info') => + computeScore([fail(id, 'src/A.svelte', sev)], config, { + rules: [ + r('x/c', 'security', 'component', 'critical'), + r('x/w', 'security', 'component', 'warning'), + r('x/i', 'security', 'component', 'info') + ], + applyCriticalCap: false + }).score; + expect(one('x/c', 'critical')).toBeLessThan(one('x/w', 'warning')); + expect(one('x/w', 'warning')).toBeLessThan(one('x/i', 'info')); + }); +}); + +describe('computeScore — inventory wiring to config (no rules option, real registry)', () => { + it('a config severity override changes the default inventory, not just buildInventory in isolation', () => { + // architecture::component is 8 info rules (inventory 8) by default. Raising one rule's severity + // to 'warning' makes it 7 info + 1 warning = 12. Verified against the real registry: a key + // failing that one rule (now 'warning', deduction 5) scores 100 - 500/12 = 58.33, floored 58. + const config = defineConfig({ rules: { 'architecture/component-size': 'warning' } }); + const results = [fail('architecture/component-size', 'src/A.svelte', 'warning')]; + expect(computeScore(results, config).score).toBe(58); + }); +}); + +describe('computeScore — a disabled rule injected via options.rules', () => { + it('scores 0 and finite for a rule turned off in config but still carried in options.rules', () => { + // `computeScore` now runs `options.rules` through `selectRules` itself, so an `off` rule never + // reaches the inventory or `pairOf`: its penalized result observes no pair, `Math.max` floors the + // denominator to `failed`, and the key scores 0 rather than NaN or a false 100. + const rule = r('a/off', 'architecture', 'component', 'warning'); + const config = defineConfig({ rules: { 'a/off': 'off' } }); + const results = [fail('a/off', 'src/A.svelte', 'warning')]; + const { score } = computeScore(results, config, { rules: [rule] }); + expect(Number.isFinite(score)).toBe(true); + expect(score).toBe(0); + }); + + it('scores 0, not 80, when the off rule shares its pair with an enabled rule', () => { + // Before `options.rules` was filtered, `buildInventory` dropped the off rule internally but + // `ruleScopes` did not: charged against the enabled rule's own inventory of 5, the same failing + // off rule scored 80 here and 0 when injected alone in its pair — an inconsistency for identical + // input. Filtering both from the same list makes the off rule invisible to both, so this must + // match the isolated case. + const off = r('p/off', 'performance', 'component', 'info'); + const enabled = r('p/warn', 'performance', 'component', 'warning'); + const config = defineConfig({ rules: { 'p/off': 'off' } }); + const results = [fail('p/off', 'src/A.svelte', 'info')]; + const { score } = computeScore(results, config, { rules: [off, enabled], applyCriticalCap: false }); + expect(Number.isFinite(score)).toBe(true); + expect(score).toBe(0); + }); +});