Skip to content

Commit 234cbc7

Browse files
ausimianclaude
andcommitted
fix: set the coverage floor across the support range, not one version
The 88.58 floor was the pinned toolchain exact reading with no slack, and cover line attribution is not constant across the versions elixir: "~> 1.18" invites in. Elixir 1.20 counts the head of defp forward_standard_error do (peer.ex:981) as executable where 1.19 does not, so the denominator goes from 473 to 474 and the same 419 covered lines report 88.40% instead of 88.58%. That failed the mandatory mix precommit on a clean tree for anyone on a current Elixir - and CI never saw it, because the test matrix runs a plain mix test while the single precommit job is pinned to 1.19. Measured, one cell per toolchain with isolated MIX_HOME and MIX_BUILD_ROOT: 1.18.4/27 81.22% 88.58% exit 0 1.19.5/28 81.22% 88.58% exit 0 1.20.3/28 80.84% 88.40% exit 0 1.20.3/29 80.84% 88.40% exit 0 The floor is now 88 - the lowest reading, rounded down so it absorbs a line of drift rather than sitting on one version figure. Negative control on 1.20.3, the least slack of the range: +1 line 88.21% exit 0, +2 88.03% exit 0, +3 87.84% exit 3. On 1.19.5 it absorbs three and fails on the fourth. Both ladders were run, not inferred. That is looser than a floor ideally is, and deliberately so: a floor that fires on a clean tree teaches people to bypass the gate. The previous comment had already said attribution varies by version while the number ignored it, so both comments are corrected too - including the one on the precommit alias, which now says plainly that this runs on whatever Elixir the contributor has. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 029bf07 commit 234cbc7

2 files changed

Lines changed: 115 additions & 41 deletions

File tree

AGENTS.md

Lines changed: 70 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1539,23 +1539,70 @@ production module spelled that way, which is the one thing an exclusion must not
15391539
do.
15401540

15411541
**It is part of `mix precommit`**, which is what makes the threshold a gate
1542-
rather than a number in a comment. Nothing else runs it, and CI's `test` matrix
1543-
stays on a plain `mix test` deliberately, because cover's line attribution can
1544-
differ between Elixir versions and the figure is measured on one toolchain — the
1545-
pinned `precommit` job is where it is enforced.
1546-
1547-
Lib-only that is **88.58%** — 419 of 473 relevant lines. Per module:
1548-
`Castle.Commands` 94.85%, `Castle` 90.48%, `Castle.Peer` 81.22%,
1549-
`Castle.Deployment` and `Castle.Error` 100%.
1550-
1551-
**The threshold is the measured figure, 88.58, and not a rounder number near
1552-
it.** One uncovered line added to `lib` gives 88.40% and fails; that was
1553-
measured rather than assumed. It replaced an 85 that sat *below* the figure it
1554-
was meant to floor, so it ratcheted nothing and licensed a thirteen-line
1555-
regression. The cost of a threshold with no slack is that a refactor which
1556-
legitimately removes covered lines fails it too, and the answer then is to
1557-
re-measure and edit the number deliberately — the same rule every other claim
1558-
here is held to.
1542+
rather than a number in a comment. Nothing else runs it: CI's `test` matrix runs
1543+
a plain `mix test`, and CI's `precommit` job is pinned to Elixir 1.19 / OTP 28.
1544+
So the *only* place this number is checked against the rest of the `~> 1.18`
1545+
range is a contributor's own machine, in the gate this project makes mandatory —
1546+
which is exactly where a false failure does the most damage, because the lesson
1547+
it teaches is to stop running the gate.
1548+
1549+
**The figure is not the same on every supported toolchain, and the threshold has
1550+
to be a floor across them rather than a reading from one.** Measured, one cell
1551+
per toolchain with its own `MIX_HOME` and `MIX_BUILD_ROOT`:
1552+
1553+
| Elixir / OTP | `Castle.Peer` | Total | relevant |
1554+
| --- | --- | --- | --- |
1555+
| 1.18.3 / 27 | 81.22% | 88.58% | 473 |
1556+
| 1.18.4 / 27 | 81.22% | 88.58% | 473 |
1557+
| 1.18.4 / 28 | 81.22% | 88.58% | 473 |
1558+
| 1.19.5 / 27 | 81.22% | 88.58% | 473 |
1559+
| 1.19.5 / 28 *(CI)* | 81.22% | 88.58% | 473 |
1560+
| 1.20.3 / 28 | 80.84% | 88.40% | 474 |
1561+
| 1.20.3 / 29 | 80.84% | 88.40% | 474 |
1562+
1563+
Same tests, same 419 covered lines, a different denominator. It tracks the
1564+
Elixir version and not OTP, and the difference is exactly one line: **1.20
1565+
counts the head of `defp forward_standard_error do` (peer.ex:981) as executable
1566+
where 1.19 does not.** Note where that lands — inside the `## In the peer`
1567+
section, so the drift went straight into the set nothing can observe, making it
1568+
34 unmeasurable lines on 1.20 rather than 33. Attribution drift is not
1569+
distributed evenly over the module, and there is no reason to expect the next
1570+
one to be either.
1571+
1572+
Per module on the pinned toolchain: `Castle.Commands` 94.85%, `Castle` 90.48%,
1573+
`Castle.Peer` 81.22%, `Castle.Deployment` and `Castle.Error` 100%.
1574+
1575+
**So the threshold is 88 — the lowest reading, rounded down.** Two earlier
1576+
values were wrong in opposite directions and both are worth keeping written
1577+
down. 85 sat *below* the figure it was meant to floor, so it ratcheted nothing
1578+
and licensed a thirteen-line regression. 88.58 was the pinned toolchain's exact
1579+
reading with no slack, and it made the mandatory `mix precommit` fail on a clean
1580+
tree under Elixir 1.20 — a false failure for anyone on a current release. The
1581+
comment beside that number had *already said* attribution varies by version; the
1582+
hazard was identified and the number ignored it, which is the more instructive
1583+
half of the mistake. Do not set this from one machine, and do not raise it to
1584+
88.40 or above: that re-creates the trap the moment another version attributes
1585+
differently.
1586+
1587+
**What 88 costs, measured on the toolchain with the least slack rather than
1588+
estimated.** Adding uncovered lines to `lib` under Elixir 1.20.3:
1589+
1590+
| added | total | exit |
1591+
| --- | --- | --- |
1592+
| 0 | 88.40% | 0 |
1593+
| 1 | 88.21% | 0 |
1594+
| 2 | 88.03% | 0 |
1595+
| 3 | 87.84% | **3** |
1596+
1597+
So it absorbs two and fails on the third; on 1.19 it absorbs three and fails on
1598+
the fourth. That is more slack than a floor ideally has, and it is the
1599+
deliberate price of enough headroom that one more line of version drift does not
1600+
produce a false failure. The direction of the trade is the point: a floor that
1601+
fires on a clean tree gets the gate bypassed, while a floor two lines loose still
1602+
catches every regression large enough to matter — and a review that let two new
1603+
uncovered branches through was not going to be saved by a percentage. Anything
1604+
tighter than about 88.2 buys back one line of sensitivity and spends the drift
1605+
headroom this paragraph exists because of.
15591606

15601607
**What cannot be measured is the peer's VM, and the reason is where
15611608
instrumentation is applied — not anything cover is unable to do.** An earlier
@@ -1567,8 +1614,9 @@ separate VM loading `Castle.Peer` from the target release's own beam files,
15671614
which nothing has instrumented. Cover's only mechanism for reaching another VM
15681615
is `:cover.start/1` over a *distributed* node, and this peer deliberately has
15691616
none. So `resolve/1` and everything below the `## In the peer` comment — 33
1570-
lines, about 7% of the shipped total — runs on every `Castle.PeerTest` and is
1571-
counted as missed, which puts the observable ceiling near 93%.
1617+
lines on 1.19 and 34 on 1.20, about 7% of the shipped total — runs on every
1618+
`Castle.PeerTest` and is counted as missed, which puts the observable ceiling
1619+
near 93%.
15721620

15731621
Do not raise it by calling those functions on the test node: that runs Elixir's
15741622
pipeline in the very VM the whole mechanism exists to keep it out of, mutates
@@ -1578,7 +1626,9 @@ worse — `{Castle.Peer, :resolve, 1}` is a contract with the *next* version of
15781626
Castle, so the MFA is not free to move for a metric.
15791627

15801628
**Why 90% is not the threshold, stated as arithmetic rather than as a claim of
1581-
impossibility.** 90% needs 426 covered, seven more than there are. Twenty-one
1629+
impossibility.** 90% needs 426 covered on 1.19's denominator of 473 — 427 on
1630+
1.20's 474 — so seven more than there are, eight on the newer attribution.
1631+
Twenty-one
15821632
missed lines are observable in principle, and they divide cleanly:
15831633

15841634
* **Five are the compiler's own generated clauses**, one line per defaulted

mix.exs

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -55,23 +55,41 @@ defmodule Castle.MixProject do
5555
# happened to be spelled that way, which is the one thing an exclusion list
5656
# must not do. Renaming a fixture makes the total drop, which is visible.
5757
#
58-
# **The threshold is the measured figure and nothing rounder.** 419 of 473
59-
# relevant lines, which is 88.58%; a single uncovered line added to `lib` takes
60-
# it to 88.37% and fails. It is set from a measurement rather than chosen, so a
61-
# refactor that legitimately removes covered lines will fail it too - and the
62-
# right answer then is to re-measure and edit this number deliberately, which
63-
# is the same rule every other claim in this project is held to. It replaced an
64-
# 85 that sat *below* the figure it was meant to floor and so licensed a
65-
# thirteen-line regression.
58+
# **The threshold is a floor across the supported range, not this machine's
59+
# reading.** `elixir: "~> 1.18"` invites in every version from 1.18 upwards,
60+
# and cover's line attribution is not the same across them: 1.18.3 through
61+
# 1.19.5 count 473 relevant lines and report 88.58%, while 1.20.3 counts one
62+
# more in `Castle.Peer` and reports 88.40%. Same tests, same covered lines, a
63+
# different denominator. So this is the *lowest* of those readings, rounded
64+
# down, so that it absorbs a line of drift instead of sitting on one version's
65+
# figure. The per-toolchain measurements are in AGENTS.md.
6666
#
67-
# 90% would need 426 covered, seven more than there are. What is left is 33
68-
# lines in the peer's VM (below) plus 21 that are observable in principle: five
69-
# are the compiler's own default-argument clauses for arities nothing calls,
70-
# and the other sixteen need a file mode, a device node, or a config provider
71-
# sabotaging Castle's working directory. So the seven would have to include all
72-
# five of the default-argument clauses, whose only effect is on this number.
73-
# That is the move this project does not make. See AGENTS.md for the line-by-
74-
# line account.
67+
# Two earlier values were wrong in opposite directions, and both mistakes are
68+
# worth keeping written down. 85 sat *below* the figure it was meant to floor,
69+
# so it ratcheted nothing and licensed a thirteen-line regression. 88.58 was
70+
# the pinned toolchain's exact reading with no slack, which made the *mandatory*
71+
# `mix precommit` fail on a clean tree under Elixir 1.20 - a false failure for
72+
# any contributor on a current release, and the comment beside it had already
73+
# said attribution varies by version while the number ignored it. So: do not
74+
# set this from one machine, and do not raise it to 88.40 or above, which
75+
# re-creates the trap the moment another version attributes differently.
76+
#
77+
# 88 absorbs two uncovered lines added to `lib` and fails on the third, which
78+
# was measured on 1.20 (the least slack of the range) rather than estimated -
79+
# 88.40, 88.21, 88.03, then 87.84 and exit 3. That is looser than a floor
80+
# ideally is, and it is the deliberate price of enough headroom that one more
81+
# line of attribution drift does not fail a clean tree. The direction of the
82+
# trade is the point: a floor that fires on a clean tree teaches people to
83+
# bypass the gate.
84+
#
85+
# 90% would need 426 covered on 1.19's denominator, seven more than there are.
86+
# What is left there is 33 lines in the peer's VM (below) plus 21 observable in
87+
# principle: five are the compiler's own default-argument clauses for arities
88+
# nothing calls, and the other sixteen need a file mode, a device node, or a
89+
# config provider sabotaging Castle's working directory. So the seven would
90+
# have to include all five of the default-argument clauses, whose only effect
91+
# is on this number. That is the move this project does not make. See AGENTS.md
92+
# for the line-by-line account.
7593
#
7694
# **What cannot be measured is the peer's VM, and the reason is where
7795
# instrumentation is applied rather than anything cover cannot do.**
@@ -87,7 +105,7 @@ defmodule Castle.MixProject do
87105
# observable ceiling near 93%.
88106
defp test_coverage do
89107
[
90-
summary: [threshold: 88.58],
108+
summary: [threshold: 88],
91109
ignore_modules: [
92110
Castle.DeploymentStub,
93111
Castle.InitStub,
@@ -123,10 +141,16 @@ defmodule Castle.MixProject do
123141
"credo --strict",
124142
# With `--cover`, so the threshold in `test_coverage/0` is a gate rather
125143
# than decoration: nothing else runs it, and a floor nothing enforces is
126-
# a number in a comment. This is the only place it is enforced - the CI
127-
# `test` matrix stays on a plain `mix test`, deliberately, because line
128-
# attribution can differ between Elixir versions and the figure is
129-
# measured on one toolchain.
144+
# a number in a comment.
145+
#
146+
# **This runs on whatever Elixir the contributor has**, which is the
147+
# thing to keep in mind before touching the threshold. CI's `test` matrix
148+
# stays on a plain `mix test` and CI's `precommit` job is pinned to one
149+
# version, so a floor set from that one version is not checked anywhere
150+
# against the rest of the `~> 1.18` range - it is checked here, on a
151+
# machine CI never sees, in the gate this project makes mandatory. That
152+
# is why the number is a floor across the range and not a reading. See
153+
# `test_coverage/0`.
130154
"test --cover"
131155
]
132156
]

0 commit comments

Comments
 (0)