fix(core): never score a key against less than 25 points of checks - #367
Merged
Merged
Conversation
Field review killed the previous design's justification: it called the model coverage while six of architecture's eight rules evaluate nothing. Making the denominator honest shrinks it, which makes every finding cost more — so the honesty fix and the severity fix pull opposite ways on the same knob. A floor of 25 settles both: it orders info below warning in every pair, removes the zero-point cases, and gives thin pairs the same number that excluding unconfigured rules would have, without making findings lighter as a project declares more. Reporting affected keys beside the score moves the magnitude signal off the score, which is what makes the floor free.
Self-review moved the checkability field: an inventory weight on ScoreModel would describe one computeScore call, but a category's call spans many keys of possibly different pairs, so there is no single weight to report there. The report gains a pair map instead. Also adds the two guards the spec's testing section required and the plan had missed — that a clean key still scores 100, and that sitePenalty stays absolute.
…t the formula The ordering test called only buildInventory and duplicated the cost formula inline, so it could never fail regardless of what computeScore did. Two re-baselined fixtures also floored identically whether their mechanism ran or not, once their inventories sat below 25. Rewrite the ordering test to score through computeScore, and re-anchor the sum-over-pairs and two-key-mean fixtures above the floor so each is exercised again.
…e the floor keeps an integral mean integral across keys was re-baselined onto PERF (inventory 9), where the deficit-space mean, a mean of key scores, and both evaluation orders all coincide — it passed but caught neither regression it exists for. Re-anchor to inventory 28 with failed weights 10 and 18, the one combination (searched, not guessed) where both mutations diverge from the true 50. Also repoints score.ts's deficit-space comment at a fixture that still exists in the file.
Adds the counter-example the design already knew about (a warning in a thin pair can outcost a critical in a thick one) so prose cannot overstate the guarantee again without a failing test.
The reporters guide and its design doc claimed a more severe finding always costs more "within a category". False: seo::component and seo::route are both seo but score independently, and a warning in the former can outcost a critical in the latter. The guarantee is per (category, scope) pair.
Two score.test.ts fixtures could no longer distinguish the behavior they were written to pin, now that the 25-point inventory floor dominates their small denominators: a config severity override changing the inventory (previously anchored below the floor at 12) and a mutant that pools every (category, scope) pair's inventory together instead of partitioning by pair (PERF's own 9-weight inventory floored regardless of what else was mixed in). Also adds a missing case: `max(inventoryWeight, failed, INVENTORY_FLOOR)` needs `failed` for keys where a penalized rule sits outside the inventory and its weight exceeds both the observed inventory and the floor at once, which no existing fixture drove.
inventoryWeight === 0 can no longer happen once INVENTORY_FLOOR sits in the same Math.max, so the ternary around the deficit division was unreachable. Note in the floor's own doc comment that this is what it guarantees, so a future change lowering the floor below 1 knows the zero-guard needs to come back with it.
Two claims about the floored-score model didn't survive the floor: - "inventories gives the divisor behind every key of a pair, so any score can be recomputed by hand" held only for routes[].categories (one pair per value). A route's own score can span more than one pair, sums their raw weights, and floors that sum once — the published (already per-pair floored) map does not reproduce it. Corrected in the reporters guide (en/ja), the JsonReport.inventories docstring, and the changeset. - "every applicable check failing scores 0" stopped being true once a key is never scored against less than 25 points of severity weight; the health score guide (en/ja) didn't mention the floor at all. Added the missing clause with a pointer to the reporters guide for the full rule. Also fixes a Japanese mistranslation: the ja reporters guide's inventories example rendered "floored" as 切り捨て (truncation) where the floor actually raises a small inventory, contradicting the body text's correct 下限値.
- "71 affected keys per displayed point at 25" was the inventory-20 figure; at 351 keys and a per-key deficit of 4, the true answer is 88. - "eight of the nine pairs sit on the floor at ~37" was off by one: seven of the nine (35, 28, 16, 9, 8, 5, 5) sit below 37, not eight. - "worst keys 87 to 96" was the single-failing-rule key; the field corpus's actual worst keys carry two failing rules, which score 75 against an inventory of 8 and 92 against the floor.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughThe PR adds a 25-point inventory floor to score calculations. It reports ChangesScoring and reporting changes
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Scoring as computeScore
participant Reporter as buildJsonReport
participant Consumer as JSON report consumer
Scoring->>Reporter: score, keys, affectedKeys
Reporter->>Reporter: build floored inventories
Reporter->>Consumer: categories and inventories
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.changeset/score-floor-and-reach.md:
- Around line 7-15: Update the release note’s opening claim to scope severity
ordering to a single category-and-scope pair, rather than comparing findings
across pairs. Revise the one-rule example to reflect the denominator formula and
state that the 80 score corresponds to a warning, while preserving the
documented scores for info and critical where applicable.
In `@docs/src/content/docs/guides/`(reporting)/health-report.md:
- Around line 19-22: Correct the all-failing score explanation in both health
reports: docs/src/content/docs/guides/(reporting)/health-report.md lines 19-22
and docs/src/content/docs/ja/guides/(reporting)/health-report.md lines 19-21.
State that the 25-point inventory floor prevents zero only for inventories below
25 points, while inventories with observedInventory at least 25 can score 0 when
every applicable check fails; preserve each guide’s language and surrounding
explanation.
In `@docs/superpowers/specs/2026-08-05-score-floor-and-reach-design.md`:
- Around line 31-33: Update the fenced code block containing inventoryWeight in
the score-floor and reach design document to specify the text language tag,
preserving the existing block content.
In `@packages/core/src/reporter/json.ts`:
- Around line 42-48: Expose sufficient raw pair and observed-pair data, or the
actual divisor, in the route-category report schema defined by
packages/core/src/reporter/json.ts:42-48 so multi-pair scores can be recomputed
with a single floor; add a mixed-scope same-category fixture and recomputation
assertion in packages/core/test/json-report.test.ts:283-297; update the
recomputability contract in
docs/superpowers/specs/2026-08-05-score-floor-and-reach-design.md:97-102 to
describe the finalized schema.
🪄 Autofix
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: 2d2679f5-46b4-41be-8f06-53d6c4176a47
📒 Files selected for processing (15)
.changeset/score-floor-and-reach.mddocs/src/content/docs/guides/(reporting)/health-report.mddocs/src/content/docs/guides/(reporting)/reporters.mddocs/src/content/docs/ja/guides/(reporting)/health-report.mddocs/src/content/docs/ja/guides/(reporting)/reporters.mddocs/superpowers/plans/2026-08-05-score-floor-and-reach.mddocs/superpowers/specs/2026-08-05-score-floor-and-reach-design.mddocs/superpowers/specs/2026-08-05-score-semantics-design.mdpackages/core/src/reporter/json.tspackages/core/src/scoring/score.tspackages/core/test/html-report.test.tspackages/core/test/json-report.test.tspackages/core/test/score.test.tspackages/vite/test/app-shell-static.test.tspackages/vite/test/ui-dashboard.test.ts
… pair "A less severe finding now costs less than a more severe one" read as a global guarantee; it only holds within one (category, scope) pair. Named the counter-example (a floored warning costs 20, a critical in seo::route costs 13.64) and the severity behind the 80 (a lone warning in a one-rule pair), reusing the reporters guide's existing wording instead of a third phrasing.
The guide claimed the inventory floor means every applicable check failing never scores 0. False once a key's own inventory reaches 25: the floor only protects a thin inventory from being zeroed by one or two findings; at or above 25 it does nothing, and a key failing everything still scores 0.
The recomputability claim held without saying why: a key is either a route id or a source file path, and those two key spaces never overlap, so a category's results on one key always draw on a single scope — the precondition routes[].categories (but not a route's own score, which can span more than one pair) recomputes from it. Stated in the inventories docstring, the reporters guide (en/ja), and the design doc's recomputability bullet.
…at spans two scopes The single-pair recompute test never exercised the case the precondition actually protects: a category whose rules span both a route and a component scope (performance and seo both do, for real, in the registry). Build one failing result per non-project registry rule and assert inventories recomputes every routes[].categories entry across the whole registry, not a hand-picked pair.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A finding labelled less severe cost more.
A key's category score is
100 − (100 × failedWeight) / inventoryWeight, where the denominator is the severity-weighted count of that category's rules at that scope. Those inventories ran from 5 to 110, so a finding's cost tracked how much its category checks rather than how severe it is. Measured on a real project of 351 keys: anarchitectureinfotook 13 points off a key while aseowarningtook 5 — on 41 of 351 keys, not a corner case. A pair holding a single rule was worse: one finding scored the key 0.The field report that surfaced this reasonably concluded the severity model was broken. It was not broken so much as undescribed — but the numbers it produced were indefensible either way.
What
A floor of 25 under the denominator.
inventoryWeight = max(observedInventory, failedWeight, 25).infowarningcriticalseo::routecorrectness::componentsecurity::componentperformance::routeseo::projectperformance::componentarchitecture::componentseo::componentperformance::projectThe worst
infonow costs 4.00 against the cheapestwarning's 4.55, soinfoorders belowwarningin every pair. The zero-point cases go with it: a lonewarningin a one-rule pair scored 0 and now scores 80.keysandaffectedKeysper category. A score is a mean over every key, so it isshare of affected keys × how badly they failed— and once the floor flattens the depth, 41 affected keys of 351 move a category by less than a point. That is what a mean of mostly-clean keys says, not a defect to tune away. Splitting the product's two factors out is what makes both legible:41 of 351distinguishes one finding from forty-one exactly, where the score cannot.An
inventoriesmap from"<category>::<scope>"to the floored weight, so the arithmetic is checkable. The field reader sawarchitecture 87, could not derive where 87 came from, and drew a conclusion about the model. Neither the inventory nor the floor appeared anywhere in the output.What is deliberately not fixed
warningis not ordered belowcriticalacross pairs, and no floor achieves it. Awarningin a floored pair costs 20; acriticalinseo::routecosts 13.64. Ordering those needs a floor near 37, where seven of the nine pairs sit on the floor and the model has become absolute deductions wearing a ratio's clothes. The case needs a thin pair carrying awarningbeside a thick pair carrying acritical, and no thin pair fired at all in the field —seo::component,performance::projectandperformance::componentwere 0 keys of 351. A test pins both halves so the limitation stays known rather than becoming accidental.Excluding unconfigured rules from the denominator was the field's own suggestion and the right instinct — six of
architecture::component's eight rules evaluate nothing. The floor reaches the same number for every pair where it would matter, without making a project's existing findings lighter as it declares more conventions.What this costs
Scores rise wherever a category checks few things. Simulated against the field's shape — 351 keys, 41 affected —
architecturegoes 98 → 99 and its affected keys 87 → 96. A--min-healthgate calibrated on the previous release will pass more easily; recalibrate it.Verification
core1232,cli805,vite206 tests pass;tsc --noEmitclean in all three after rebuilding core;pnpm smoke8/8; lint and format clean.Every guard was confirmed load-bearing by mutation: the floor (12 tests), the evaluation order (1), the deficit-space mean (2), the
(category, scope)pair in both directions (16 scope-only, 14 category-only),max(…, failed)(1), the inventory's dependence onconfig.rules(1), and the cap's decision on the raw value (1). The invariant was attacked seven ways —treatDynamicAs: 'warn'including a promotion above a thin inventory, a penalized result whose rule is absent from the inventory, an empty set, a category with only project-scoped findings — and held each time.The interesting part: nine tests that passed while holding nothing
Adding a floor collapses every fixture anchored below it onto the same number. Tests keep passing and stop discriminating.
Nine instances, found in four separate passes: three by the implementer during the first task, one by a task reviewer sweeping all 75 tests, one in a plan fixture that turned out arithmetically impossible, three more by the final whole-branch review — including one gutted by the previous release rather than this one — and a ninth by the implementer's own vacuity sweep, where the
max(…, failed)guard turned out to have no test at all: removing it failed nothing.The same pass caught two false published claims. One said any score could be recomputed by hand from
inventories; it holds forroutes[].categoriesand not forroutes[].score, because the code sums raw per-pair inventories and floors once while the map publishes floored values — a two-pair key scores 80 where hand-summing the map gives 90. The other was a pre-existing guide sentence saying a key with every applicable check failing scores 0, which this branch falsified without updating it.One earlier claim in this line of work is worth recording because of how it survived: the design asserted a more severe finding always costs more within a category. That is false —
seospans two pairs, and awarningin the thin one costs 20 against acritical's 13.64 in the thick one. It passed four adversarial spec review passes and was transcribed faithfully into two languages before a reviewer checked it against running code. The guarantee is per(category, scope)pair, and the docs now say so in both languages.Design:
docs/superpowers/specs/2026-08-05-score-floor-and-reach-design.md, superseding2026-08-05-score-semantics-design.md, which is on this branch marked withdrawn with its measurements intact. Plan:docs/superpowers/plans/2026-08-05-score-floor-and-reach.md.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation