feat(core): score a category by how much is wrong, not whether anything is - #363
Merged
Merged
Conversation
Six findings, all reproduced: two table values were rounded (the operation the predecessor spec exists to forbid); the model was undefined at the three call sites that pass multi-category results; 100*(1-f/i) loses a displayed point on integral values; 0/0 reached clamp as NaN; the route mean's exact-integer premise no longer holds; the cli call-site count was wrong.
The clean-project argument for deficit space was false — a mean of exact 100s is exact under either arithmetic, so the test it implied could not fail. The real case is an exactly-integral mean of unclean keys, and deficit space narrows that class without closing it; both are now stated with fixtures.
computeScore now measures each route/file against the inventory of rules that could apply to its (category, scope) pair, instead of a flat 100-point start with fixed per-severity deductions. This lets a key reach 0 and keeps its magnitude proportional to how much of its pair actually failed, rather than compressing everything into a few fixed steps. Also removes the DEDUCTION duplication between score.ts and inventory.ts: it now has a single declaration, exported from inventory.ts.
…t it actually protects The zero-denominator case was already handled by the `inventoryWeight === 0` ternary below it, not by this guard. Without `max`, a penalized finding whose weight exceeds its own pair's inventory (treatDynamicAs: 'warn' promoting severity, or a rule absent from the inventory) would push a key's deficit negative and silently score 100 despite a real finding.
…sted guards The f=88,i=110 anchor for `keeps an integral score integral` passes under both wrong evaluation orders (100*(1-f/i) and 100*(f/i)): the deficit-space mean's subtraction cancels their fp error at that value, so the test held nothing. Add f=28,i=50 (single key) and a two-key mean at shared inventory 11, both verified to discriminate; keep the old anchor as a non-load-bearing example. Also add: a computeScore test that a config.rules severity override reaches the default inventory (not just buildInventory called directly), and a test for the `?? 0` guard on inventory.get(p) — the real NaN guard for a rule that's off in config but still carried through options.rules.
It described the applied-rules semantics the score-proportionality design
rejected ("share of the checks that ran ... that passed"). The denominator is
the inventory of the (category, scope) pairs the route touched, which
includes selected rules that had nothing to say about that particular route.
…lity design selectRules and buildInventory only read top-level config.rules, so an overrides entry that raises a rule's severity for a glob leaves the denominator at the rule's base severity while the result carries the raised one — a third way failedWeight can exceed observedInventory, alongside the two already recorded. Measured on the eight-info architecture pair: 0 when raised only in overrides, 31.8 when the same raise is made at top level.
|
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 (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe PR replaces fixed per-finding deductions with severity-weighted proportional category scores. It adds category/scope inventories, updates ChangesProportional scoring
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant computeScore
participant selectRules
participant buildInventory
participant ruleScopes
participant RouteResults
computeScore->>selectRules: select configured or supplied rules
computeScore->>buildInventory: build category/scope inventory
computeScore->>ruleScopes: map rule IDs to category/scope pairs
computeScore->>RouteResults: group route pairs and calculate weighted deficits
RouteResults-->>computeScore: return route scores
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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
🧹 Nitpick comments (1)
.changeset/score-proportionality.md (1)
10-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid a fixed rule count in the release note.
The statement "
architectureis eightinforules" will become stale when rules are added or removed. Use durable wording, or state that eight is only the count for this release.
Based on learnings, avoid hard-coded counts for extensible rule sets; use durable wording unless this count is a fixed contract.🤖 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 @.changeset/score-proportionality.md around lines 10 - 12, Update the release note’s explanation around the architecture category to remove the hard-coded “eight” rule count, using durable wording that describes the cap without specifying an extensible rule total.Source: Learnings
🤖 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-proportionality.md:
- Around line 18-22: Update the score-change description in
score-proportionality.md to use conditional, project-dependent wording: scores
and health-gate outcomes may change under the new model, but clean reports can
retain a displayed score of 100 and the magnitude or direction of changes is not
guaranteed for every project. Preserve the existing explanation of recalibrating
--min-health and the routes[].score meaning.
In `@docs/superpowers/plans/2026-08-04-score-proportionality.md`:
- Around line 159-163: Add a dated supersession note near severityOf documenting
that the shipped implementation returns undefined for an off injected rule and
skips it, resulting in the expected inventory of 15. Keep the historical code
block unchanged, and reference packages/core/src/scoring/inventory.ts plus its
regression test as the implementation and verification sources.
In `@packages/core/src/scoring/score.ts`:
- Around line 45-47: Filter the selected rules before both buildInventory and
ruleScopes in the score flow around options.rules, ensuring disabled injected
rules are excluded from pairOf as well as the inventory. Preserve scoring so a
disabled rule producing a finding scores 0 in mixed enabled/disabled pairs, and
add a regression test covering an off info rule paired with an enabled warning
rule.
In `@packages/core/test/score.test.ts`:
- Line 360: Rename the tests in packages/core/test/score.test.ts at lines
360-360 and 379-379 to describe the preserved score behavior, removing
“load-bearing,” anchor, and evaluation-order rationale; update line 465-465 to
describe the zero-inventory behavior instead of the “?? 0” NaN guard. No
assertion or implementation changes are needed.
---
Nitpick comments:
In @.changeset/score-proportionality.md:
- Around line 10-12: Update the release note’s explanation around the
architecture category to remove the hard-coded “eight” rule count, using durable
wording that describes the cap without specifying an extensible rule total.
🪄 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: 4aea75e8-ba39-42c4-9f2d-b3e05fb53ab4
📒 Files selected for processing (12)
.changeset/score-proportionality.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-04-score-proportionality.mddocs/superpowers/specs/2026-08-04-score-proportionality-design.mdpackages/core/src/scoring/inventory.tspackages/core/src/scoring/score.tspackages/core/test/health.test.tspackages/core/test/inventory.test.tspackages/core/test/score.test.ts
`buildInventory` filtered `off` rules; `ruleScopes` did not. An injected disabled rule therefore mapped to a pair it contributed no weight to, so the same finding scored a key 80 when another rule shared its pair and 0 when it did not. Both helpers now take the same selected list.
…gences "Every score changes" was false for a clean report, which still displays 100. The plan's Task 1 also contradicted itself on disabled rules, and the design described the rules escape hatch as taking a list as given.
This was referenced Aug 6, 2026
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 category score moved with whether something was wrong, not with how much.
Two symptoms, both measured. They share one cause.
A category could not reach most of its own scale. A key — a route or a source file — lost a fixed number of points per failing rule, and is only ever touched by rules of its own scope. So the lowest score a key could reach was fixed by that scope's rule inventory:
Architecture is eight
inforules and nothing else, so an architecture score was a nine-value scale presented as a hundred-value one. Three more scopes sat above 90 — this was not one miscalibrated category, it is what the model did wherever a scope's rules were few or cheap.The average erased magnitude. One
infofinding moves a mean of N keys by1/N. At the scale a field measurement reported — 585 keys — one finding and 276 findings both displayed 99.Fixing either symptom alone fixes neither. Raising the deductions leaves architecture bounded; un-bounding architecture without making each affected key cost more leaves the mean pinned near 100.
This is the follow-up the previous scoring change recorded against itself: "Trading '100 is a lie' for '99 says nothing' is the right trade only if the second half gets fixed."
What
A key now scores the share of what it was measured against that is intact:
inventoryWeightismax(observedInventory, failedWeight), whereobservedInventorysumsDEDUCTION[severity]over the selected rules whose(category, scope)pair is among the pairs observed on that key. Keys are averaged in deficit space. Every category can now reach 0, and the 276-finding case reads 94 against 99 for one finding.seoandcorrectness— the largest inventories, and the categories users read most — land within a point of their old values. The change concentrates where the scale was most compressed.Decisions that are load-bearing rather than stylistic
The denominator is the scope's inventory, not the rules that applied. The faithful "not applicable" semantics collapses here: measuring each architecture rule's
appliescondition shows an ordinary component has one or two applicable architecture rules, so a props-less component a few lines over the size threshold would score 0. Flooring the denominator does not rescue it — at 300 keys with 10 affected, a per-key 0 yields a category mean of 96.7 and a per-key 80 yields 99.3, so the larger the floor the less of the fix survives.The pair, not the scope alone.
computeScoretakes results, not a category, and three call sites pass multi-category sets —routes[].score, the console's route tree, and the Vite plugin. Summing over observed(category, scope)pairs defines those and reduces to the single-category rule exactly when the input is single-category.Unchanged on purpose:
sitePenaltystays absolute points (neither symptom applies to it — it is never divided by the key count), acriticalstill caps a category at 79, and a displayed 100 still means no finding among the checks that ran.No signature cascade.
computeScorederives the inventory itself fromselectRules(allRules, config);ScoreOptionsgains an optionalrulesfor tests. Nothing undersrc/rules/imports anything undersrc/scoring/, so the direction is free. Deliberately unlike the previous change that threaded a rule-id list throughAnalyzeResult: there the CLI's--categorynarrowing was needed; here it would be wrong, since--category seomust not shrinkseo's own inventory.The arithmetic, which is where this could be wrong while looking right
Three forms were mandatory, each because the alternative loses a displayed point:
100 − (100 × f) / i, never100 × (1 − f / i)— the second yields19.999999999999996forf = 88, i = 110and displays 19 for a true 20.49.99999999999999for two keys atf = 3andf = 6againsti = 9, displaying 49 for a true 50.max(observedInventory, failedWeight)plus a zero guard — without them a result whose rule is absent from the inventory producesNaN, which reaches the JSON reporter asnull.The design was reviewed adversarially three times and rejected twice before approval. That process caught, among others, two table values computed with rounding — the operation the predecessor design exists to forbid — and a justification for deficit space that was arithmetically false: a clean project's mean is exactly 100 under either form, so the test that justification implied could not fail. The real case is an exactly-integral mean of unclean keys. One residual is stated rather than smoothed: a multi-key exactly-integral mean may display one point low, bounded at one point and verified by exhaustive and randomized sweeps against exact rational truth.
Verification
core1210,cli805,vite206 tests pass; typecheck clean in all three after rebuilding core; lint and format clean.Every guard was confirmed load-bearing by mutation, not assumed: the evaluation order, the deficit-space mean,
max, the zero guard, the?? 0on the inventory lookup, the pair in both directions, and the inventory's dependence on configuration.That last set exists because the whole-branch review found the opposite. The test written to hold the evaluation order held nothing — mutating the shipped code to either wrong form left all 1206 tests green, because the deficit-space mean turns
100 − 19.999999999999996into exactly 80, rounding away the very error the anchor was chosen to expose. It is now anchored onf = 28, i = 50(44 shipped, 43 mutated), with the original kept as a labelled non-load-bearing example. Two further guards were found untested the same way and now have tests.Migration
Every score changes, most of them downward, and by more than a point. A
--min-healthgate calibrated against the old numbers will start failing; recalibrate against the new scale.routes[].scorein the JSON report changes meaning the same way. Stored baselines are unaffected — they key on findings, not scores.Out of scope, found while reviewing
pnpm smoke's JSON check fails deterministically onmainas well as here: the fixture report is 67,656 bytes, past the 64 KiB pipe buffer, and the analyze path callsprocess.exitafterconsole.log. Reproduced againstmainat 6174836. It needs its own issue and is untouched by this branch.Design:
docs/superpowers/specs/2026-08-04-score-proportionality-design.md. Plan:docs/superpowers/plans/2026-08-04-score-proportionality.md.🤖 Generated with Claude Code
Summary by CodeRabbit
--min-healththresholds after upgrading.