fix(core): make a displayed score of 100 mean zero findings - #337
Conversation
Three corrections, all from review of the first draft: - health floored the already-rounded category scores, so the two rounding stages composed and moved it by two points — breaking the very bound the change rests on. It now floors the raw category means once. - deleting unit-entry-file's pass would have removed the only per-rule evidence that the rule ran, which zero findings cannot distinguish from a declaration matching nothing. The pass stays and loses its route instead, which is what created the score key. - the invariant is bounded by which checks actually ran, since L3 rules are inert until declared; claiming more would invite the same complaint this spec exists to prevent.
Review caught that exposing computeScore's unrounded route mean would drop sitePenalty and CRITICAL_CAP out of Health — so a project with only site-wide findings would show Health 100, breaking this spec's own invariant, and a category capped at 79 by a critical would not move Health at all. computeScore now exposes its unrounded final score. Two consequences follow and are recorded: the cap must be decided on the raw value so it cannot disagree with the floored one at the boundary, and scoreModel.routeAverage can floor for display without breaking score = routeAverage - sitePenalty, since sitePenalty is an integer.
- Testing 5 still asserted the rule emits nothing for a conforming unit, which the revised design contradicts and which would have failed the intended implementation. It now asserts the absence of a route. - dropping location as well as route would collapse every pass to one findingKey and delete the pass from every --diff run, since filterToChangedFiles keeps only results whose location git listed. location plays no part in the denominator, so it stays. - the family-verifiability gap was described as recorded below but was not in the follow-up list; it is now the third item. - the JSON channel still cannot answer 'did this rule run', so per-rule counts in summary are recorded as the follow-up that should go first.
The boundary example did not demonstrate its own claim: both cap decisions yield the same displayed score, and they cannot differ, since a disagreement would require raw - floor(raw) > 1. The divergence is in the raw score Health averages — deciding on the floored mean can leave a capped category contributing nearly a point above the cap. Also: the follow-up list said two items and holds four; CRITICAL_CAP is unchanged in value and effect but not in decision input, which an implementer reading only the testing line would miss; and score = routeAverage - sitePenalty holds only where neither the cap nor the clamp binds, so a test must not assert it unconditionally.
… decision The prior fixture (200 keys, one critical) had rawRouteAverage 99.925, where both a raw-value and a floored-mean cap decision bind, so the test could not catch a regression to deciding on the floored value. Replaced it with a 10-key fixture whose raw mean (79.9) sits strictly between 79 and 80, where the two decisions disagree.
… Passed location, floor the by-route tail computeHealth's floor(weighted / total) could publish 99 for a project with zero findings: fractional weights (e.g. 0.1) leave ~1e-14 of representation error in the weighted sums, and unlike Math.round that used to absorb it, Math.floor turned an exact 100 into 99. Guard with + 1e-9; computeScore's routeAverage needs no such guard since route scores are integers and division of exact integers is exact when the quotient is an integer. console.ts's verbose Passed listing rendered r.route only, so architecture/unit-entry-file's route-less passes (Task 3) all printed as identical, path-less lines — render r.location ?? r.route instead. The --by-route hidden-tail average still used Math.round, which could claim "avg score 100" over a tail containing a penalized route; changed to Math.floor for consistency with the rest of the branch. Docs: health-report.md (en/ja) stated the "at most 99" invariant without its weight-0 exception (a category weighted 0 is excluded from Health and can carry a critical without lowering it, already pinned by an existing test), and the worked example (Health: 85 over SEO 90 / Performance 75) was arithmetically unreachable even under the old rounding. Added the exception and corrected the example to 82. Also pinned two testing-plan assertions for architecture/unit-entry-file that were implemented but unverified: summary.passed still counts checked units, and --diff keeps a pass for a changed entry file while dropping it for an unchanged one.
📝 WalkthroughWalkthroughThe PR changes score calculations from rounding to flooring, exposes raw scores for Health aggregation, excludes zero-weight categories, changes conforming unit-entry results from ChangesScore honesty
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant RouteFindings
participant calculateScore
participant computeHealth
RouteFindings->>calculateScore: route findings and site penalty
calculateScore->>calculateScore: compute raw score and floor display score
calculateScore->>computeHealth: category rawScore
computeHealth->>computeHealth: apply weighted deficits and floor Health
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-07-31-score-honesty-design.md`:
- Around line 100-103: Update
docs/superpowers/specs/2026-07-31-score-honesty-design.md:100-103 to state the
Health 100 invariant only for present categories with positive effective weight.
In docs/superpowers/plans/2026-07-31-score-honesty.md:21-25, retain the
historical wording and add a dated, scoped supersession note documenting the
zero-weight exception and referencing the current specification and test. In
packages/core/test/health.test.ts:143-146, rename the test to describe
positively weighted categories and add a zero-weight category containing a
penalized finding while asserting Health remains 100.
In `@packages/core/src/scoring/score.ts`:
- Around line 144-150: Update the weighted health calculation near the score
aggregation and the relevant scoring function so it does not use a fixed 1e-9
epsilon with unrestricted positive weights; compute the weighted deficit
directly or enforce a minimum weight ratio, ensuring any applicable finding
keeps --min-health 100 from passing. Add a regression test covering a raw score
of 99 with weight 1e-12 alongside a clean category with weight 1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 57a7073d-d913-4e19-9346-b5c5ad1ddf0c
📒 Files selected for processing (14)
.changeset/score-honesty.mddocs/src/content/docs/guides/(reporting)/health-report.mddocs/src/content/docs/ja/guides/(reporting)/health-report.mddocs/superpowers/plans/2026-07-31-score-honesty.mddocs/superpowers/specs/2026-07-31-score-honesty-design.mdpackages/cli/test/changed-files.test.tspackages/core/src/reporter/console.tspackages/core/src/rules/architecture/unit-entry-file.tspackages/core/src/scoring/score.tspackages/core/test/console-report.test.tspackages/core/test/health.test.tspackages/core/test/score.test.tspackages/core/test/unit-entry-file-example.test.tspackages/core/test/unit-entry-file.test.ts
…guard Math.floor(weighted / total + 1e-9) assumed the smallest real difference between categories was 1/N, which holds only for comparable weights. config.weights accepts any finite non-negative value, so a category can be weighted arbitrarily small (e.g. 1e-12) and still hide a real finding behind the epsilon while --min-health 100 passes. Average the weighted deficit (100 - rawScore) instead of the weighted score. A clean project sums to exactly 0 with no floating-point error, so the zero case needs no tolerance; Math.min(99, ...) then makes "any finding means at most 99" structural rather than arithmetic. Also qualifies the "displayed 100" invariant, in the design spec and a health.test.ts case, to categories with a positive effective weight — a weight-0 category is present but excluded from the average and can carry a critical finding without moving Health. The plan doc gets a dated follow-up note rather than a rewrite, since it is a historical record. The public guides already stated this exception and are unchanged.
There was a problem hiding this comment.
Pull request overview
This PR tightens scoring semantics in @svelte-vitals/core so that a displayed score of 100 is only possible when no deductions occurred, fixing cases where large numbers of low-severity findings could still display as “perfect”.
Changes:
- Floor (not round) category scoring and Health, and expose an unrounded
rawScoreto avoid compounded rounding in Health. - Rework Health aggregation to average deficits (based on
rawScore) and ensure “any finding ⇒ Health ≤ 99” for positively weighted categories. - Prevent
architecture/unit-entry-filepassing results from seeding new score keys by droppingrouteon passes (keepinglocationfor traceability and diff-scoping).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/scoring/score.ts | Floors category scoring, adds rawScore, and updates Health computation to avoid double-rounding and enforce the “100 means zero deduction” invariant. |
| packages/core/src/rules/architecture/unit-entry-file.ts | Stops conforming passes from contributing new score keys by removing route and keeping location. |
| packages/core/src/reporter/console.ts | Floors the hidden tail average in --by-route output; improves verbose passed output for route-less passes by showing location. |
| packages/core/test/score.test.ts | Adds regression/boundary tests for flooring, rawScore, and cap decision behavior; updates prior expectations. |
| packages/core/test/health.test.ts | Adds boundary and regression coverage for single-stage flooring and deficit-based Health averaging, including fractional weights. |
| packages/core/test/console-report.test.ts | Pins console output so tail averages can’t claim 100 when hidden routes are penalized; verifies route-less pass rendering. |
| packages/core/test/unit-entry-file.test.ts | Updates pass assertions (route-less, location-only) and adds scoring/summary invariants for route-less passes. |
| packages/core/test/unit-entry-file-example.test.ts | Updates example expectations to identify examined units via location and correct inert-key filtering logic. |
| packages/cli/test/changed-files.test.ts | Adds coverage ensuring --diff filtering can keep/drop unit-entry-file passes based on changed entry files. |
| docs/src/content/docs/guides/(reporting)/health-report.md | Updates guide text to reflect flooring and single-stage Health computation (needs a wording correction; see comments). |
| docs/src/content/docs/ja/guides/(reporting)/health-report.md | Japanese mirror of the Health guide updates (needs a wording correction; see comments). |
| docs/superpowers/specs/2026-07-31-score-honesty-design.md | Adds the approved design spec documenting the rounding/flooring invariant and rationale. |
| docs/superpowers/plans/2026-07-31-score-honesty.md | Adds the implementation plan that maps tasks to code/tests/docs. |
| .changeset/score-honesty.md | Adds a patch changeset describing the user-visible scoring shift and the unit-entry-file denominator fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| The result is **floored**, not rounded to nearest, so a displayed score of 100 means the deduction was | ||
| exactly zero. A category or project with any finding at all — even a single `info` — scores at most 99, | ||
| in any category you have not weighted `0`. A category weighted `0` is excluded from the Health average | ||
| entirely, so it can carry findings — even a `critical` — without pulling Health below 100. |
There was a problem hiding this comment.
Correct, and fixed in 9ecd96d. Verified in the code rather than reasoned about: config.weights is read only inside computeHealth (packages/core/src/scoring/score.ts:127) — computeScore, which produces every category score, never touches it. So a category score is weight-independent and 0 cannot raise it.
My sentence attached the weight condition to the wrong quantity ("scores at most 99, in any category you have not weighted 0"), which reads exactly as you describe. The two statements are now separate:
- any finding puts its category's score at 99 or below — no weight qualifier, because none applies;
- weights change Health only, and
0leaves the category out of that average — which is why Health can read 100 while a0-weighted category displays a score of its own,criticalfindings and all.
Mirrored in the Japanese guide with the same split.
| **結果に存在するカテゴリだけが平均に入ります。** 例えば、パフォーマンスルールに1件も該当しないプロジェクトでは、パフォーマンスカテゴリは結果を生成せず、Health は残りのカテゴリだけから算出されます。 | ||
|
|
||
| 結果は最も近い整数に丸められます。 | ||
| 結果は最も近い整数への丸めではなく、**切り捨て**られます。したがって表示スコアが 100 であれば、減点が正確にゼロだったことを意味します。`info` が1件でもあれば、重み `0` を設定していない限り、そのカテゴリ(またはプロジェクト全体)のスコアは最大でも 99 です。重み `0` のカテゴリは Health の平均から完全に除外されるため、`critical` な検出結果があっても Health を 100 未満には引き下げません。 |
There was a problem hiding this comment.
ご指摘のとおりで、9ecd96de で修正しました。コードで確認したところ、config.weights を読むのは computeHealth(packages/core/src/scoring/score.ts:127)だけで、カテゴリスコアを算出する computeScore は一切参照していません。したがってカテゴリスコアは重みと無関係で、0 にしてもスコアは上がりません。
元の文は重みの条件を係る先を間違えており(「重み 0 を設定していない限り…最大でも 99」)、まさにご指摘の読み方になっていました。2つの主張を分離しました。
- 検出結果が1件でもあればそのカテゴリのスコアは 99 以下 — 重みの条件は付きません(該当しないので)
- 重みが変えるのは Health のみで、
0はそのカテゴリを平均から外す — だからこそ重み0のカテゴリがcriticalを含むスコアを表示していても Health は 100 になりえます
英語版も同じ分け方で揃えています。
The paragraph attached the weight condition to the category score, so it read as though weighting a category 0 could put its own score above 99. computeScore never reads config.weights — only computeHealth does — so a category score is weight-independent, and 0 changes nothing about it except that Health leaves the category out of its average.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/scoring/score.ts (1)
124-154: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrevent extreme valid weights from bypassing the Health invariant.
Line 132 can underflow a positive weighted deficit to
0. For example, a positive subnormal weight with a categoryrawScorebelow100can produceweightedDeficit === 0, so Line 154 returns100.Lines 132-133 can also overflow with multiple
Number.MAX_VALUEweights. This can makeaverageDeficitNaNor0.Normalize weights by the largest positive present weight before accumulation. Also track whether any positive-weight category has
rawScore < 100. If that condition is true, cap Health at99even when floating-point arithmetic loses the deficit.Add regression tests for a subnormal positive weight and multiple maximum finite weights. This preserves the PR requirement that a positive-weight finding fails
--min-health 100.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/scoring/score.ts` around lines 124 - 154, The weighted health calculation in the scoring flow must prevent extreme finite weights from bypassing the 99-point cap. In the loop building weightedDeficit and total, normalize each weight by the largest positive present weight before accumulation, track whether any positively weighted category has rawScore below 100, and use that flag when computing health so such findings never return 100 even if arithmetic underflows or overflows; preserve the empty-category and all-zero-weight behavior. Add regression tests covering a positive subnormal weight and multiple Number.MAX_VALUE weights, verifying a finding cannot satisfy a 100 minimum.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/core/src/scoring/score.ts`:
- Around line 124-154: The weighted health calculation in the scoring flow must
prevent extreme finite weights from bypassing the 99-point cap. In the loop
building weightedDeficit and total, normalize each weight by the largest
positive present weight before accumulation, track whether any positively
weighted category has rawScore below 100, and use that flag when computing
health so such findings never return 100 even if arithmetic underflows or
overflows; preserve the empty-category and all-zero-weight behavior. Add
regression tests covering a positive subnormal weight and multiple
Number.MAX_VALUE weights, verifying a finding cannot satisfy a 100 minimum.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5907e657-01a2-49e0-8e9d-5e15b7925371
📒 Files selected for processing (6)
docs/src/content/docs/guides/(reporting)/health-report.mddocs/src/content/docs/ja/guides/(reporting)/health-report.mddocs/superpowers/plans/2026-07-31-score-honesty.mddocs/superpowers/specs/2026-07-31-score-honesty-design.mdpackages/core/src/scoring/score.tspackages/core/test/health.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/superpowers/specs/2026-07-31-score-honesty-design.md
- packages/core/test/health.test.ts
- docs/superpowers/plans/2026-07-31-score-honesty.md
Ruling 2's carve-out documentation claimed architecture/unit-entry-file's route-less PASS was score-inert in both directions under --diff. An independent review empirically refuted this: scoresByCategory buckets every result by category regardless of route, so keeping this PASS on a changed file moves --diff Health 79 -> 89 (verified on both builds) -- the same absent-to-fabricated-100 promotion this PR eliminates elsewhere. Only the narrower claim (no per-route-key deficit contribution) was true. The fix's behavior is unchanged -- main's filter already kept this PASS unconditionally, so this is a pre-existing #337 tradeoff being preserved, not a new one. Corrects the record in four places: the spike's unit-entry-file exception paragraph, changed-files.ts's doc comment, and both changesets (the cli changeset's inert claim, and the core changeset's inaccurate "JSON report's passing entries" wording plus an added note on the console reporter's --verbose display change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s:-scoped overrides reach them (#416) * fix(core,cli): give PASS results uniform location attribution so files:-scoped overrides reach them Implements docs/superpowers/specs/2026-08-08-pass-result-location-design.md, option (a): every PASS result now carries the same `location` its penalized counterpart would. core: 18 PASS-emitting rule files gain a `location` on their PASS branch (the spike's original 13-row blast-radius table plus 9 more rule ids the table's grep missed -- performance/lcp-image, render-blocking-script, font-preload-crossorigin, preload-missing-as, image-dimensions, image-loading-hint, responsive-image, seo/image-alt, architecture/private-scope-import -- verified against live source and folded into the table). Fixes issue #382: a `files:`-scoped `severity: 'off'` override can now remove a rule's passing seed, not just its penalized findings. cli: `filterToChangedFiles` is redefined to keep a result only when it's penalized, or is architecture/unit-entry-file's deliberately route-less pass seed (PR #337) -- otherwise a route-carrying PASS now uniformly located would leak into `--diff`/`--staged` and inflate a category from absent to a fabricated 100. This also fixes a pre-existing, undetected instance of the same leak for `seo/title-presence` and the ten `headTagRule`-backed rule ids, which already carried `location` on PASS before this change. Measured on the reference shape: --diff Health 89 -> 79 (79 is correct). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: fix changeset accuracy per review — scores move via the 'off' fix itself; note action propagation Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: correct unit-entry-file 'score-inert' claim per independent review Ruling 2's carve-out documentation claimed architecture/unit-entry-file's route-less PASS was score-inert in both directions under --diff. An independent review empirically refuted this: scoresByCategory buckets every result by category regardless of route, so keeping this PASS on a changed file moves --diff Health 79 -> 89 (verified on both builds) -- the same absent-to-fabricated-100 promotion this PR eliminates elsewhere. Only the narrower claim (no per-route-key deficit contribution) was true. The fix's behavior is unchanged -- main's filter already kept this PASS unconditionally, so this is a pre-existing #337 tradeoff being preserved, not a new one. Corrects the record in four places: the spike's unit-entry-file exception paragraph, changed-files.ts's doc comment, and both changesets (the cli changeset's inert claim, and the core changeset's inaccurate "JSON report's passing entries" wording plus an added note on the console reporter's --verbose display change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Why
A field measurement on a real SvelteKit app found a category carrying 276 findings while displaying a score of 100.
That is not a rounding inaccuracy.
100reads as "nothing wrong", and it was printed for a tree with 276 things wrong. The Health number is the product's capstone, so a figure that says "perfect" while findings exist undermines the one number users are meant to trust.The report could not tell, from the output alone, whether
infofindings were excluded from category scores by design or were being lost somewhere — and asked. They are not excluded:computeScorededucts for every severity. The cause wasMath.roundover a large denominator.Reproduced at the reported scale — 585 score keys, one
infofinding on N of them:It took 293 findings to move the number off 100, and a finding on every single key still showed 99.
What
The invariant this establishes: a displayed 100 means the deduction was exactly zero.
computeScorefloors its route average and its final score, and additionally returns the unrounded final score.computeHealthaverages those unrounded scores and floors once. It previously averaged integers that had each already been rounded, so the two stages composed: raw category scores[99.9, 99.9, 99.9, 99.9, 97.9]gave 100 under the old code and 98 under a naive two-stage floor — a two-point move from a change whose premise is that the difference is at most one. Flooring once gives 99.architecture/unit-entry-file's pass keeps itslocationand loses itsroute.routeis what seeded a score key, and for a plain.tsunit entry that is a key no other rule produces — a real project declaring.tsunits gained 109 fresh100s in its denominator, diluting every genuine finding.Three alternatives rejected during review, recorded so they are not re-proposed
sitePenaltyandCRITICAL_CAPout of Health entirely: a project whose route keys are clean but which has site-wide findings would show Health 100, breaking this change's own invariant.unit-entry-filepass only for.svelteentries.svelte—architecture/component-size'sappliesisc.loc > 0, so an unparseable component produces no key. A rule cannot reason about other rules'appliesconditions.CRITICAL_CAP's value and effect are unchanged (a capped category still displays 79), but its decision input moves from the rounded mean to the raw one. Deciding on the floored value would let a capped category contribute nearly a point above the cap to Health.User-visible effect
Every score moves down by 0 or 1 point. If you gate CI with
--min-healthat or just above your current score, lower the threshold by one.--min-health 100now fails on any finding at all, which is the honest reading of 100.What this deliberately does not fix
Findings are still not proportional to the score. After flooring, a category with 1 finding and one with 276 both display 99, because a single finding moves a mean of N keys by
1/N. This change restores the top of the scale, not the middle of it — the most harmful lie a headline number can tell is "perfect" when it is not, and separating the two changes keeps it clear which produced any observed difference. Review made the sharper point that this trades "100 is a lie" for "99 says nothing", so the follow-up is recorded as raised in priority, not lowered.Three further follow-ups are recorded in the design doc, including that
--reporter jsonstill cannot answer "did this rule run" — the channel the field test and CI both use.Verification
core1142,cli780,vite205,mcp25 tests pass; typecheck clean in all four packages; lint and format clean.Every mechanism has a test that fails without it, verified by mutation rather than assumed. Two are worth naming because they were written after review found the originals proved nothing:
rawScore(79 capped vs 79.9 uncapped) — the displayed score cannot discriminate at all, since that would requireraw - floor(raw) > 1;Design:
docs/superpowers/specs/2026-07-31-score-honesty-design.md(four review rounds). Plan:docs/superpowers/plans/2026-07-31-score-honesty.md.One bug this change introduced and then fixed
The final whole-branch review caught that
Math.floor(weighted / total)floors a floating-point quotient: with a fractional weight,110 / 1.1 === 99.99999999999999, so--weights seo=0.1,performance=1reported Health 99 for a project with zero findings — the invariant broken in the opposite direction.Math.roundhad absorbed the ~1e-14 error;floorpublished it as a whole point. Every weight in the test suite was an integer, which is why four per-task reviews missed it. Guarded with+ 1e-9, which is a thousand times smaller than the smallest real difference (1/Nfor a million keys).computeScoreneeds no such guard and deliberately does not get one — its route scores are integers, so an integral quotient is exact.🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Changes