Skip to content

ci(test): enforce coverage floors at current levels#2659

Closed
jordan-simonovski wants to merge 1 commit into
mainfrom
jordansimonovski/coverage-floors
Closed

ci(test): enforce coverage floors at current levels#2659
jordan-simonovski wants to merge 1 commit into
mainfrom
jordansimonovski/coverage-floors

Conversation

@jordan-simonovski

Copy link
Copy Markdown
Contributor

Jest already collects coverage but nothing gates on it, so coverage can decay unnoticed. This pins per-package coverage thresholds just below the current measured levels, so the floor can only rise — decay becomes a build failure.

What changed

  • Added coverageThreshold.global (statements/branches/functions/lines) to the unit jest.config.js of packages/app, packages/common-utils, and packages/hdx-eval, each set to the measured value minus ~1 point.
  • packages/hdx-eval's ci:unit gained --coverage so its threshold is actually collected.

Key decisions

Impact

  • A change that drops coverage below the floor fails the unit job.
  • Tests/config only — no changeset.
  • Follow-up to track: there's no collectCoverageFrom, so a brand-new entirely untested file doesn't lower the global percentage and won't trip the floor. Adding it later would re-baseline all thresholds; worth a tracked issue rather than expanding this PR.
Implementation detail

Verified both directions: make ci-unit passes with the floors; temporarily setting statements: 99 in common-utils fails with a threshold error; reverted.

Pin Jest coverageThreshold in the unit configs for app, common-utils and
hdx-eval to just below each package's measured coverage (measured minus one,
rounded down). Coverage can now only ratchet upwards: any decay below these
floors fails the build, so the floors rise only when someone raises them
deliberately. Also add --coverage to hdx-eval's ci:unit so its threshold is
actually collected and enforced.

Integration configs are left untouched. The API unit config is deferred to a
follow-up until the API unit/int split (#2653) lands.
@vercel

vercel Bot commented Jul 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Jul 16, 2026 5:27am
hyperdx-storybook Ready Ready Preview, Comment Jul 16, 2026 5:27am

Request Review

@changeset-bot

changeset-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 5cd967b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added the review/tier-3 Standard — full human review required label Jul 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

  • Cross-layer change: touches frontend (packages/app) + shared utils (packages/common-utils)

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats
  • Production files changed: 4
  • Production lines changed: 35
  • Branch: jordansimonovski/coverage-floors
  • Author: jordan-simonovski

To override this classification, remove the review/tier-3 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR pins Jest coverageThreshold.global floors in three packages (app, common-utils, hdx-eval) just below their currently measured levels, and adds --coverage to hdx-eval's ci:unit script so its threshold is actually enforced in CI. The intent is to make coverage decay a build failure while avoiding any immediate red build.

  • packages/app, packages/common-utils: coverageThreshold.global blocks added; both packages already pass --coverage in ci:unit, so the gate is immediately live.
  • packages/hdx-eval: --coverage added to ci:unit alongside the new threshold; without this flag the threshold block would be silently ignored.
  • Known gap (acknowledged in PR): no collectCoverageFrom is configured, so entirely-untested new files are excluded from coverage accounting and won't trip these floors — a tracked follow-up rather than scope for this PR.

Confidence Score: 5/5

Config-only change that adds coverage floors; no production code touched and no risk of breaking existing behaviour.

All three packages already collected coverage in CI (or now do, after adding --coverage to hdx-eval), so the thresholds will fire correctly. The floors are set below current measured values, so today's build passes. The only known gap — new untested files escaping detection without collectCoverageFrom — is explicitly documented and deferred, not an oversight that could cause a silent regression.

No files require special attention; the change is mechanical and the one acknowledged gap is already tracked for a follow-up.

Important Files Changed

Filename Overview
packages/app/jest.config.js Adds coverageThreshold.global with whole-number floors (statements 53, branches 37, functions 38, lines 54); ci:unit already carries --coverage so the gate is live.
packages/common-utils/jest.config.js Adds coverageThreshold.global with the highest floors of the three packages (statements 86, branches 78); ci:unit already carries --coverage so the gate is live.
packages/hdx-eval/jest.config.js Adds coverageThreshold.global (statements 70, branches 55, functions 72, lines 71); threshold depends on the --coverage flag added to package.json.
packages/hdx-eval/package.json Adds --coverage to ci:unit, making coverageThreshold active in CI; dev:unit watch script intentionally left without --coverage (expected behaviour).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ci:unit runs] --> B{--coverage flag?}
    B -- yes --> C[Jest collects coverage]
    B -- no --> Z[coverageThreshold silently ignored]
    C --> D{coverageThreshold defined?}
    D -- yes --> E[Compare measured % vs floor]
    D -- no --> F[Build passes regardless]
    E --> G{All metrics >= floor?}
    G -- yes --> H[Build passes]
    G -- no --> I[Build fails with threshold error]

    subgraph packages_app ["packages/app"]
        PA["ci:unit: jest --ci --coverage\nstatements>=53 branches>=37\nfunctions>=38 lines>=54"]
    end
    subgraph packages_common_utils ["packages/common-utils"]
        PCU["ci:unit: jest --runInBand --ci --coverage\nstatements>=86 branches>=78\nfunctions>=85 lines>=86"]
    end
    subgraph packages_hdx_eval ["packages/hdx-eval (--coverage added in this PR)"]
        PHE["ci:unit: jest --ci --coverage --forceExit\nstatements>=70 branches>=55\nfunctions>=72 lines>=71"]
    end

    PA --> A
    PCU --> A
    PHE --> A
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[ci:unit runs] --> B{--coverage flag?}
    B -- yes --> C[Jest collects coverage]
    B -- no --> Z[coverageThreshold silently ignored]
    C --> D{coverageThreshold defined?}
    D -- yes --> E[Compare measured % vs floor]
    D -- no --> F[Build passes regardless]
    E --> G{All metrics >= floor?}
    G -- yes --> H[Build passes]
    G -- no --> I[Build fails with threshold error]

    subgraph packages_app ["packages/app"]
        PA["ci:unit: jest --ci --coverage\nstatements>=53 branches>=37\nfunctions>=38 lines>=54"]
    end
    subgraph packages_common_utils ["packages/common-utils"]
        PCU["ci:unit: jest --runInBand --ci --coverage\nstatements>=86 branches>=78\nfunctions>=85 lines>=86"]
    end
    subgraph packages_hdx_eval ["packages/hdx-eval (--coverage added in this PR)"]
        PHE["ci:unit: jest --ci --coverage --forceExit\nstatements>=70 branches>=55\nfunctions>=72 lines>=71"]
    end

    PA --> A
    PCU --> A
    PHE --> A
Loading

Reviews (1): Last reviewed commit: "ci(test): enforce coverage floors at cur..." | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

This is a test-infrastructure-only change (three jest.config.js coverage floors plus --coverage added to hdx-eval's ci:unit). No production code path is touched, no changeset is required (the repo's standards explicitly exempt CI/test config), and both app and common-utils already collected coverage so their new thresholds are enforced immediately. The one substantive concern is the gating mechanism's fidelity, below.

🟡 P2 -- recommended

  • packages/app/jest.config.js:31 -- no collectCoverageFrom is set in any of the three configs, so the coverage denominator is only the source files a test transitively imports; the floor therefore does not measure package coverage and the "can only ratchet up" guarantee has two holes.
    • Fix: add a collectCoverageFrom glob covering each package's source tree so a brand-new untested file lowers the global percentage and a refactor that changes which files get imported cannot flip the floor spuriously.
    • ce-correctness-reviewer, ce-testing-reviewer
🔵 P3 nitpicks (2)
  • packages/hdx-eval/jest.config.js:19 -- floors are pinned ~1 point below measured, and branch coverage (branches: 55 here, branches: 37 in app) is the metric most sensitive to compiler/runtime variance, so a legitimate refactor could trip the gate with no behavioral regression.
    • Fix: confirm the numbers hold on the CI runner's Node version and widen the branch buffer slightly if margins are tight.
  • packages/common-utils/jest.config.js:21 -- the identical floor-comment block and the hardcoded per-metric numbers are duplicated across three configs with no single source of truth, so they will drift from measured coverage over time.
    • Fix: consider a shared base jest config or a script that reports the current gap, and treat the inline "never lower silently" comment as convention only.

Reviewers (5): ce-correctness-reviewer, ce-testing-reviewer, ce-maintainability-reviewer, ce-project-standards-reviewer, ce-learnings-researcher.

Testing gaps:

  • Verify each pinned floor was measured with the exact CI command (npx nx run-many -t ci:unit) before merge — hdx-eval's --coverage is newly added, so if any metric was rounded up its ci:unit fails deterministically on the first CI run.
  • packages/api (deferred) and packages/cli (skipped) remain outside the ratchet, so decay in those packages is ungated until the API unit split lands.
  • common-utils's ci:int (jest.int.config.js) carries no threshold, so integration-only-covered code is not ratcheted.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 239 passed • 3 skipped • 1614s

Status Count
✅ Passed 239
❌ Failed 0
⚠️ Flaky 2
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

@jordan-simonovski

Copy link
Copy Markdown
Contributor Author

Thanks — no code changes here; the P2s are known/deferred:

  • No collectCoverageFrom (denominator is only imported files): real limitation, tracked as an explicit follow-up. Adding it re-bases every threshold (the denominator grows), so it's a deliberate separate change rather than scope creep here. This PR pins the floor against the existing collection method so decay of tested code is caught today.
  • Branch coverage sensitive to variance: the measured − 1 buffer absorbs minor movement; if a floor ever trips on a legitimate refactor, the fix is to lower that one number (a reviewable ratchet-down), not remove the gate.
  • Duplicated comment/numbers across three configs: inherent to per-package Jest configs; a shared preset is possible later but would couple the packages' test config.

@jordan-simonovski

Copy link
Copy Markdown
Contributor Author

Folded into #2654 (combined quality-gate floors PR) via cherry-pick — coverage floors + ESLint ceilings ship together as one 'metrics can only improve' gate. Closing to halve merge overhead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-3 Standard — full human review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant