Skip to content

ci: let Codecov fail a bad change instead of commenting - #340

Merged
lens0021 merged 1 commit into
mainfrom
claude/codecov-status
Aug 15, 2026
Merged

ci: let Codecov fail a bad change instead of commenting#340
lens0021 merged 1 commit into
mainfrom
claude/codecov-status

Conversation

@lens0021

@lens0021 lens0021 commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Both statuses were informational: true, which is the "never fail" setting, so the only thing Codecov did was post a comment on every pull request. This turns the comment off and gives both statuses something they can actually fail on.

Where the numbers come from

Codecov's app and API are not reachable from here, but the codecov/patch check run records the project total as its auto target, so the history is readable off GitHub:

PR date check output
#226 2026-07-19 0.00% of diff hit (target 13.62%)
#285 2026-08-12 34.75% of diff hit (target 19.25%)
#326 2026-08-15 0.00% of diff hit (target 24.90%)

So the project total is around 25%, and it moved five points in three days. maintenance/ is 2561 of the 4932 PHP lines outside tests/, and it is essentially all uncovered — #326 was six lines of maintenance/buildTranslations.php at 0%, and #285's misses were 74 in storeWebfonts.php, 9 in rewriteScripts.php, 2 in buildScripts.php, 1 in build.php. That is what holds the total at 25%, and it is why an absolute project target is the wrong instrument: any number above today's coverage fails immediately, and any number below it stops being a check the moment coverage rises.

What the config does now

Project — target: auto, threshold: 2%. Compare against the pull request's base rather than a fixed line, and allow a two-point dip. The allowance is sized to the thing that legitimately moves the total: a new maintenance script of 200-400 lines at 0% coverage costs roughly 1-2 points with nothing having regressed. A real regression is much larger — dropping the tests for one mid-sized class under includes/ is worth four or five points, well past the allowance. The trade-off is honest: losing coverage of one small class (~40 statements, ~1.8 points) would slip through. Tightening to 1% closes that but fails on the next maintenance script, which is the failure mode you asked me to avoid.

Note that removed_code_behavior is left at its adjust_base default, so deleting covered code does not itself count as a regression.

Patch — target: 70%, scoped to includes/. A patch target applied to the whole tree can never pass on a maintenance/-only change — #326 would have failed at any target above zero. Restricting the status to includes/ makes it a real bar for the code that is actually unit-testable, and leaves script changes alone (Codecov passes a status whose paths the diff does not touch). Checked against history: #285 would pass, since its includes/ lines were 42/43 in Webfonts.php plus 0/5 in WikvenSettings.php, or 87.5%; #326 and #336 touch no includes/ lines and pass trivially. 70% rather than something higher leaves room for a couple of uncovered guard clauses. The known annoyance is a very small includes/ diff — four lines with two uncovered is 50% and fails — so 60% is the knob if that shows up in practice.

I did not add an ignore: block for maintenance/. It would raise the reported total sharply and make the patch scoping unnecessary, but it also changes what the README badge means, which is a bigger call than this change.

The project status has never actually existed

Worth knowing before relying on it: codecov/project has never appeared on this repository, on any surface, at any point since coverage was introduced.

Checked both GitHub surfaces, because they are different lists:

What that rules out:

  • Not informational. Both statuses carried it, and patch posted anyway.
  • Not a missing base report or an unresolvable total. The patch status's auto target is the base commit's project coverage, and it has resolved to a real number every time (13.62%, 19.25%, 24.90%). Codecov computes this repository's project coverage on every pull request — it simply never publishes a status for it.
  • Not the fixes: rewrite or the upload shape. Per-file patch coverage resolves to real repository paths, so the path rewrite and the clover upload are both working.
  • Not branch protection. Protection can require or hide a check, but it cannot stop one being created, and no codecov/project is ever created.

Also consistent: no Codecov comment on this repository has ever carried a project-coverage line — every one of them reports diff coverage only. Project-level output appears to be switched off account-side rather than repository-side, which would explain the status and the comment together.

That leaves causes outside this file, and both need the Codecov dashboard to confirm — see the review thread. Until then the project block here may simply be inert, and codecov/patch is the status that actually enforces anything.

Verification

Both statuses were informational, so nothing could fail and the bot
narrated every pull request instead.

The project status now compares against the base with a 2% allowance,
and the patch status wants 70% on includes/ only. maintenance/ is half
the tree's lines and carries no unit tests, so it drags the project
total down by a point or two whenever a script is added and would fail
any patch target outright.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016qiZSGWHrxkjvFxA8swynp
@lens0021

Copy link
Copy Markdown
Collaborator Author

Following up on the missing codecov/project, since it is the other half of "fail the check when coverage gets worse".

First, a correction to how I reported it

My earlier note said "no codecov/project check run exists". That was true but phrased in a way that invited the obvious objection — that I had only looked at the Checks API, while Codecov historically posted codecov/project and codecov/patch as commit statuses, which is a different endpoint. So I went back and checked both, on head SHAs across the whole life of the integration:

surface endpoint #226 #285 #326 #336 #340
check runs GET /commits/{sha}/check-runs patch only patch only patch only patch only patch only
commit statuses GET /commits/{sha}/status total_count: 0 total_count: 0 total_count: 0 total_count: 0 total_count: 0

There are no commit statuses on this repository at all — not from Codecov, not from anything. Everything reports through the Checks API. So the finding holds on both surfaces, and it was not an artefact of which list I read. Worth having stated explicitly either way.

What the evidence rules out

  • informational is not the cause. Both statuses carried informational: true and codecov/patch posted regardless. A setting that suppressed posting would have suppressed both.
  • A missing base report is not the cause. The patch status's auto target is the base commit's project coverage, and it resolved to a real figure every single time — 13.62% on fix: inline static bundle icons as data URIs so subpages don't 404 #226, 19.25% on feat: carry ULS webfonts in the export, opt in #285, 24.90% on fix: drain translation jobs by hand again, reverting #311 #326. Codecov computes this repository's project coverage on every pull request. It just never publishes a status carrying it.
  • The fixes: rewrite and the upload shape are not the cause. Patch coverage resolves to real repository paths (maintenance/buildTranslations.php, includes/Webfonts.php, …), so the in-container path rewrite works and the clover upload is being read.
  • Branch protection is not the cause. Required-check settings can require or hide a check, but cannot prevent one being created — and nothing named codecov/project is ever created. (A required check that never reports would also sit visibly as "Expected"; none does.)
  • A YAML validation failure is not the cause. If .codecov.yml were rejected, Codecov would fall back to its defaults, and the defaults post both statuses. We get patch only, which is not the default shape.

What is left, and where to look

The one other consistent symptom: no Codecov comment on this repository has ever included a project-coverage line. #226, #285, #321, #326 and #336 all report diff coverage and a file table, and none of them carries the usual Project coverage is X%. Comparing base … to head … line. The status and the comment are missing the same thing, which points at project-level output being switched off account-side rather than anything in this file.

Two candidates, both needing the dashboard — app.codecov.io and codecov.io are blocked from where I work, so I could not check either and I am not going to guess between them:

  1. Plan tierapp.codecov.io/plan/gh/chaotic-ground. Codecov's lower tier is documented as diff/patch coverage only, with project-coverage status checks and project coverage in PR comments as paid-tier features. If that is what this org is on, it explains the status and the comment together and there is nothing to change in .codecov.yml.
  2. Account-level YAMLapp.codecov.io/account/gh/chaotic-ground/yaml. A global YAML with coverage: status: project: false would override the repository file. Worth reading even if it looks empty.

The single most useful page is the repository's YAML tab — app.codecov.io/gh/chaotic-ground/wikven/settings/yaml. It shows the effective merged config Codecov actually used, plus any validation errors. That distinguishes the two cleanly:

  • merged YAML shows a project status, but no check is posted → plan tier (candidate 1)
  • merged YAML has project stripped out → something upstream is removing it (candidate 2)

That page also answers the one thing I could not verify about this PR at all: whether Codecov's parser accepts paths: and threshold: as written.

Consequences either way

  • If project is unavailable on this account, the project: block in .codecov.yml is inert. It is harmless to leave — it costs nothing and starts working the day the account can post it — but the enforcement this PR actually buys you is codecov/patch at 70% on includes/. That is worth knowing rather than assuming both halves are live.
  • If it turns out informational was somehow implicated after all, merging this fixes it by itself. The cheap test: after this merges, watch the first pull request that touches PHP and see whether codecov/project shows up. That costs nothing and settles it without the dashboard.

One thing I could not check at all

Whether the Codecov checks are in the repository's required-checks list. I have no read access to branch protection or rulesets through this session's GitHub app, and a failing check only blocks a merge if it is required. If codecov/patch is not on that list, this PR makes the check go red without stopping anything. Worth confirming in Settings → Branches (or Rules) while you are looking at the rest.

No repository-side fix to open a PR for — as far as GitHub can show, this is not something .codecov.yml can reach.


Generated by Claude Code

@lens0021
lens0021 marked this pull request as ready for review August 15, 2026 13:59
@lens0021
lens0021 merged commit e25f4d7 into main Aug 15, 2026
22 checks passed
@lens0021
lens0021 deleted the claude/codecov-status branch August 15, 2026 13:59
lens0021 added a commit that referenced this pull request Aug 15, 2026
Project coverage turns out to be a Codecov Pro feature, and this account
is not on it — which is why `codecov/project` has never appeared on any
pull request here. The block was describing an enforcement that cannot
exist, so it goes.

`comment: false` and the `patch` status are unchanged. Patch at 70% on
`includes/` is the check that actually runs.

Follow-up to #340; the full investigation is in [that
thread](#340 (comment)).

`uv run --frozen --group lint yamllint --strict .` passes. Codecov's own
validator is still unreachable from here, so the file is unvalidated
against their parser.

---
_Generated by [Claude
Code](https://claude.ai/code/session_016qiZSGWHrxkjvFxA8swynp)_

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants