mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
Maximilian Roos
4202cffe61
## What prompted this Getting #3605 green ran into codecov reporting a `base_commit` three commits older than the real merge-base. This audits whether our config causes that. ## The cause Codecov picks a PR's base by walking back to the newest ancestor that has a coverage report. It used the real merge-base for PRs #3480, #3532 and #3602, and a stale one for #3603 and #3605. The difference is whether the merge-base uploaded a report. **29 of the last 40 main commits did not.** `ci` had one concurrency group for main pushes, and GitHub cancels the *pending* run in a group whenever a newer one joins, even with `cancel-in-progress: false`. So the question is how long a run holds the group, and a run isn't done until its slowest job is: | job | duration on main | |-----|------------------| | `fast-checks` | 2 min | | `code-coverage` | 3-4 min | | `test (windows)` | 11 min | | `collect affected coverage (windows)` | 110-129 min | Each main run held the group for ~2 hours, so nearly every subsequent main push was cancelled while queued, taking the 4-minute coverage job with it. Every cancelled main run's `updated_at` lands within a second of the next push's `created_at`. The 2 hours is real work, not queue: 2-5s from `created_at` to `started_at`, then 108 minutes inside `cargo affected collect` — 4181 tests under `-C instrument-coverage` with a per-test LLVM profile, ~5 GB of profraw. ## The fix: one workflow per cadence The three groups of jobs have incompatible needs, and one group was serving all of them. | workflow | cadence on main | why | |----------|-----------------|-----| | `ci` | every commit, ~11 min | required gate + fast checks | | `coverage` | every commit, keyed per-sha | a skipped upload leaves later PRs on a stale base | | `affected` | sampled, ~2 h | a DB a few commits old still anchors a correct superset | `affected` keeps exactly the grouping it has today, so its sampling is unchanged and deliberate. It just no longer drags the other two along. ### Scope of the impact The posted `codecov/patch` check scopes to the PR's own GitHub diff, so a stale base did **not** score PRs against other people's lines. On #3605 the posted 91.66% is exactly `github.rs`'s 11/12, while the stale-base compare object reported 64/65 across 13 files. What a stale base costs: - `codecov/project` reports "compared to \<stale sha\>" - the patch `auto` target is the stale base's project coverage (0.02pp here) - the compare API object widens to `base..head`, which is what made the investigation look like silence Separately, `test`/`lint`/`fast-checks` also stopped completing on main. Nothing load-bearing rode on that (they already ran on the PR), but it left `tend-ci-fix` with nothing to watch, since it doesn't fire on cancelled runs. ## Two smaller fixes - `ignore: "**/tests/**"` compiles to `.*/tests/.*` (confirmed against codecov's validator), which needs a leading directory and so never matched `tests/` itself. Inert today since `cargo llvm-cov` reports only `src/` (verified against a downloaded `cobertura.xml`), but now correct if that changes. Now `tests/**`. - `fail_ci_if_error` gated on `github.repository_owner`, which is the *base* repo's owner on a fork PR too, so the soft-fail its comment describes never applied. It keys off the head repo now. ## Docs The API behaviour was ours to misuse, not codecov's to explain. Three traps, all confirmed against the live API: - `file_report/<path>/` 404s with `coverage info not found` because the route swallows the trailing slash into the path. Without it the endpoint returns `line_coverage`. - `?pullid=N` always compares the PR's **current** head. `?base=&head=` asks about an earlier commit. - the compare response has no `patch_totals` key, and `.name` is `{base, head}` rather than a string, so a filename lookup silently matches nothing. A working recipe already existed in `running-tend`, but that skill is scoped to CI. `tests/CLAUDE.md` owns coverage investigation, so the queries go there and `running-tend` points at them instead of keeping a second copy. Re-running the corrected query against #3605's failing commit reproduces the miss exactly: `src/git/remote_ref/github.rs:164`, the `gh repo set-default` hint, matching what the session eventually found by hand. ## This PR demonstrates it It changes no Rust at all, only YAML and markdown. Codecov still reported a **10-file, 111-line patch** on its first commit, because it based the comparison on `203603909` rather than the real merge-base `32f380a27`. Every main commit in between has no report: | commit | ci run | report | |--------|--------|--------| | `32f380a27` | queued | no | | `9645e3e13` | cancelled | no | | `bcd1ffdfd` | cancelled | no | | `8865f20ab` | cancelled | no | Every one of those 111 patch lines belongs to somebody else's merged commit. It passed at 100% only because those commits are well covered. > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
21 lines
588 B
YAML
21 lines
588 B
YAML
comment: false
|
|
|
|
# `cargo llvm-cov` reports only `src/`, so this is a guard rather than a live
|
|
# filter. It has to be `tests/**`: codecov compiles `**/tests/**` to
|
|
# `.*/tests/.*`, which needs a leading directory and so misses `tests/` itself.
|
|
ignore:
|
|
- "tests/**"
|
|
|
|
coverage:
|
|
status:
|
|
project:
|
|
default:
|
|
removed_code_behavior: adjust_base
|
|
# This disables report a success/failure. That's not helpful on `main`
|
|
# and we get the success/failure from the patch status on PRs.
|
|
informational: true
|
|
|
|
patch:
|
|
default:
|
|
only_pulls: true
|