feat(rules)!: deprecate the hygiene score and stop it certifying empty scans (META-284) - #42
Conversation
…y scans (META-284)
Implements ADR-003 amendment A-002, ratified 2026-08-03.
computeHygieneScore([], 0) returned { value: 100, grade: 'A' }. No findings
meant no penalty, no penalty meant a full score, a full score meant an A, and
coverageRatio was computed but never consulted by the scoring path. A scan that
observed nothing certified a repository as flawless, and that value reached a
published artifact.
The function returns HygieneScore | null now, and null when nothing was
observed. null is not a bad grade; it is the absence of one, and a caller has to
handle it rather than inherit an A. coverageRatio is number | undefined, because
the old 0 was the default parameter arriving unchanged at every call site rather
than a measurement.
Both are source-level breaks for TypeScript readers, which is the intended
alarm: that is exactly the code that would otherwise read absence as a pass.
AuditResult.score is nullable for the same reason.
Nothing is removed and no schema bytes change. ADR-003 §5 gives a
normative-optional field a deprecation notice and a documented migration now,
with removal at the next declared breaking boundary; the profile stays at 0.4,
so this release is not it. generated.hygiene stays declared while a first-party
producer still emits it — emission ceases first, then the field and exports go
together.
The old behavior was pinned by a test named 'clean repo scores 100'. An empty
findings array is not a clean repository; it is an absence of evidence, and the
two were indistinguishable in the return value.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
qmarcelle has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Reviewer's GuideDeprecates the hygiene score by making it nullable when a scan observes nothing, distinguishes unmeasured coverage from zero coverage, and propagates these type and behavior changes through the rules engine, tests, audit types, and release metadata with deprecation guidance. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
HygieneScoretype definescoverageRatio?: number | undefined;, which is redundant given the optional property marker; consider simplifying this tocoverageRatio?: numberto avoid confusion and better reflect the intendedundefined-for-unmeasured semantics. - The
observedNothingcheck treatstotalRepoFiles === 0the same as an undefined denominator, so a repo with zero files and no findings yieldsnull; if a zero-file repo should be considered a fully observed scan, you may want to distinguish that case explicitly (for example, by treatingtotalRepoFiles === 0as a valid denominator rather than as absence).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `HygieneScore` type defines `coverageRatio?: number | undefined;`, which is redundant given the optional property marker; consider simplifying this to `coverageRatio?: number` to avoid confusion and better reflect the intended `undefined`-for-unmeasured semantics.
- The `observedNothing` check treats `totalRepoFiles === 0` the same as an undefined denominator, so a repo with zero files and no findings yields `null`; if a zero-file repo should be considered a fully observed scan, you may want to distinguish that case explicitly (for example, by treating `totalRepoFiles === 0` as a valid denominator rather than as absence).Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
The three coverage states — not measured, measured as zero, measured as a ratio — were stacked in a nested ternary. Written as a guard instead, so each state is visible on its own line. No behavior change; build, typecheck and tests pass unchanged.
There was a problem hiding this comment.
qmarcelle has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Review-gate dispositionRequired checks: all green. SonarCloud: red, and dispositioned rather than fixed. Full detail so this is reviewable rather than asserted. Fixed
Not fixed — false positive by constructionThree remaining, all
S1874 flags usage of a deprecated API. A deprecation's own declaration site and implementation necessarily use the thing being deprecated. There is no arrangement of this code that both deprecates One alternative was considered and rejected: move Effect on the gateThese three alone hold SonarCloud is not a required status check on |
There was a problem hiding this comment.
qmarcelle has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
… exhausted (#43) The protocol document still named Greptile the required automated status gate and told agents not to merge while its review was pending. That requirement cannot be satisfied and would block every merge indefinitely. It also recorded the wrong cause. It said the app was 'likely not installed'; the app IS installed and does respond, with a 50-credit trial-limit notice and zero check runs. Observed on PR #37 on 2026-08-13, and again on PR #42, which carries the notice twice. An uninstalled app is a setup gap; an exhausted quota is a reviewer that answers and says nothing. A quota notice is never a pass. Three claims corrected against measured state: * Greptile as required status check -> withdrawn 2026-08-13, verified against branch protection, which requires exactly test (20), test (22) and Four-path producer conformance * 'Require code owner review: currently required' -> disabled; require_code_owner_reviews is false * the Greptile-premised remediation sequence -> both of its first two steps have already executed, and the outcome is that no reviewer requirement remains at all docs/repository-settings.md already recorded all of this accurately on 2026-08-13. This document had drifted away from its own sibling, which is the defect: two files in one repository disagreeing about whether a merge gate exists. The .greptile/ rules are kept. They are correct and cost nothing while dormant, and credits are the only thing standing between them and running again.
Implements ADR-003 amendment A-002, ratified 2026-08-03 (disposition: Remove; authority: sole steward). The ADR that
docs/versioning.mdrequires before a breaking change already exists, so this is execution rather than a fresh decision.The defect
computeHygieneScore([], 0)returned{ value: 100, grade: 'A' }. No findings meant no penalty, no penalty meant a full score, a full score meant an A — andcoverageRatiowas computed but never consulted by the scoring path. A scan that observed nothing certified a repository as flawless, and that value reached a published artifact.What changed
computeHygieneScorereturnsHygieneScore | null.nullwhen the scan observed nothing: no findings, and no denominator to say anything was examined.nullis not a bad grade — it is the absence of one, and a caller has to handle it rather than inherit anA. Where evidence exists, the arithmetic is untouched.coverageRatioisnumber | undefined. It was0whenever no total was supplied, which is every current call site — that zero was the default parameter arriving unchanged, never a measurement. "Not measured" and "zero" no longer share a value.AuditResult.scoreisHygieneScore | null, so a caller handed no evidence has somewhere truthful to put that. The previous non-nullable field left fabricating a perfect score as the only way to satisfy it.Deprecations on all three surfaces, each carrying the migration. Nothing the migration needs is private —
Finding.state,.severity,.confidenceand.temporalWeightare the only inputs the function ever had.Why
minorand notpatchBoth type changes are source-level breaks for TypeScript readers: code assigning the result to a bare
HygieneScore, orcoverageRatioto a barenumber, stops compiling. That is the intended alarm — it is exactly the code that would otherwise read absence as a pass. Same shape as A-009 and A-010, which were also minors for the same reason.changeset statusstill resolves to 0.5.0. This does not move the release number.What is deliberately NOT here
Nothing is removed and no schema bytes change. ADR-003 §5 gives a normative-optional field a deprecation notice and a documented migration now, with removal at the next declared breaking boundary. The document profile stays at
generated.specVersion: "0.4", so this release is not that boundary.generated.hygieneremains declared in the schema, because a first-party producer still emits it — removing the declaration while that is true would describe the artifact incorrectly. Emission ceases first, on the producer's schedule, and the field and exports go afterwards.Downstream, and intended
Two call sites in
workspacejson/cliwill stop compiling when it bumps to 0.5.0:packages/cli/src/producer/generate.tsandpackages/agents-audit-compat/src/audit.ts. Both callcomputeHygieneScore(run.findings)with no denominator. The migration is not merely to handlenull— it is to passrepo.files.length, which is already in scope at the producer's call site, so coverage becomes a measurement for the first time; then emitgenerated.hygienewhere a score exists and omit the optional field where it does not.A test asserted the defect
it('clean repo scores 100')pinnedcomputeHygieneScore([])atvalue: 100, grade: 'A'. An empty findings array is not a clean repository; it is an absence of evidence, and the two were indistinguishable in the return value. Replaced by four tests encoding the new truth, including the exact input traced in META-284.Verification
All fourteen standard gates pass locally: architecture + red tests, docs, ADR index, build, typecheck, test, schema provenance, examples, path-identity corpus, packed tarballs + red tests, clean-room consumer install.
Summary by Sourcery
Deprecate the hygiene score while making empty scans and unmeasured coverage explicit instead of representing them as successful results.
Bug Fixes:
Enhancements:
Tests: