Skip to content

fix(gates): an empty ADR-020 diff scope is na, not structural (#268) - #270

Merged
rubenvdlinde merged 1 commit into
mainfrom
fix/gate-268-empty-diff-scope-is-na
Aug 8, 2026
Merged

fix(gates): an empty ADR-020 diff scope is na, not structural (#268)#270
rubenvdlinde merged 1 commit into
mainfrom
fix/gate-268-empty-diff-scope-is-na

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Fixes #268.

The regression

#258 correctly stopped gates 19/25/62/63 printing PASS over a scope they never opened — a real false green. But it filed the empty-diff-scope case as structural, and structural counts against --require-full-coverage. So the run exited 98 and the PR failed for a gate that had nothing to judge.

The category was the bug, not the skip. The runner's own definitions already distinguish them, and the summary header has always read "subject matter absent from this repo or this diff":

category meaning counts against coverage
na absent from this repo or this diff. No change the author could make would put a spec file into a diff that does not touch one. no
structural the subject matter EXISTS and nothing produced the gate's input — a gap the repo can close (the axe-report case it was written for). yes

Gates 4, 6, 7 and 28 already called the identical situation na ("0 lib/Controller PHP file(s) in this diff"). The four touched by #258 were the outliers. They now match.

I audited every remaining structural call site — 24, 28 (×2), 30, 33. Each is a genuine gap inside a non-empty scope, and all five keep their category. No other site makes this conflation.

#258's fix is intact, because it lives in the rendering and not the accounting: an unopened scope prints NOT APPLICABLE, which is not PASS.

Measured

Package-vs-package on unchanged trees. The emitted gate count is identical on both sides — nothing was widened away, only the denominator moved:

repo old package this branch gates green
doriath #191 98 0 57 → 57
doriath development tip 4bfeab7c8 98 0 53 → 53
larpingapp b0c60f734 98 0 53 → 53
softwarecatalog 31d87d1 98 0 57 → 57

Why gate-63 is na on a controller-only change, and not something to "reconcile"

It was proposed that gate-63 should treat a changed SettingsController.php as its subject. It should not, and the source settles it: every rule in check_settings() reads a manifest or src/menu-layout.json. ADR-079 is about where a settings surface is declared, not what a settings endpoint does. lib/Controller/ is not even lib/Settings/, which is the only PHP path the gate consults (as a boolean, in a WARN).

There is no single global scope that gates "agree" on — each gate declares its own inputs. On doriath#191 the controller is judged, by the ten gates whose subject it is: 5, 6, 7, 8, 9, 14, 16, 25, 47, 48, all PASS.

And this widening has already been tried and reverted. check_store() carries the record:

This scan used to walk the whole lib/ tree unconditionally while the manifest half above honoured ADR-020. […] One pre-existing violation in pipelinq therefore blocked EVERY manifest-touching PR in that repo, permanently, with a finding naming a file outside the diff.

Tests

test_gate_empty_scope_never_passes.sh grows from three arms to five, pinning both directions:

  • ARM 1 planted true positives still caught full-tree (anti-widening)
  • ARM 2 empty scope is NOT APPLICABLE, never PASS, and does not exit 98
  • ARM 3 a clean in-scope manifest still PASSes
  • ARM 4 a genuinely structural gap (--axe-enabled, no report) still exits 98
  • ARM 5 a real ADR-079 violation in an in-scope manifest still FAILs — na must not swallow a subject that is in the diff

Mutation-checked, each mutation asserting old in source before rewriting so a mutation that never applied cannot masquerade as a survivor:

mutant killed by
M1 revert gate-63 to structural ARM 2
M2 gate-33's real structural gap → na ARM 4
M3 gate-63 declares na unconditionally ARM 5 only

M3 is why ARM 5 exists: ARM 3 does not catch it, because a clean manifest still returns rc=0 and passes.

Also fixes _verdict(), which parsed "NOT APPLICABLE" as "NOT" under a single-token pattern — the assertions were failing on the string, not the behaviour.

Full package suite: 41 helper suites + 59 entry-point tests, 0 failures; the 2 pre-existing quarantines untouched.

The larger finding in #268 — partly addressed, rest left open

"The strict-subset rule is comparing a live measurement against a 30-second-old build of a different program."

Correct. This PR emits the gate package's own SHA into the run output. doriath's development tip was gated green 8 seconds before the classification change landed on main — the difference between "green" and "red" was the gates, not the code, and no log said so.

This does not fix the unsoundness; it makes it checkable, which is the prerequisite for any fix. Resolution order is HYDRA_GATES_PKG_SHA → the package's git checkout → VERSION → a loud UNKNOWN, never silence (a missing line is indistinguishable from an older package that never had one).

The policy question stays open in #268: pin consumers, re-run the baseline with the PR's package, or record the package in the baseline artifact. That is a fleet-wide CI decision, out of scope for this fix, and deliberately not silently dropped.

#258 correctly stopped gates 19/25/62/63 printing PASS over a scope they never
opened. But it filed the empty-diff-scope case as `structural`, and
`structural` counts against --require-full-coverage — so the run exited 98 and
the PR failed for a gate that had nothing to judge.

The category was the bug, not the skip. The runner's own definitions already
distinguish them, and the summary header has always read "subject matter absent
from this repo OR THIS DIFF":

  na          absent from this repo or this diff. Nothing is missing, and no
              change the author could make would put a spec file into a diff
              that does not touch one.
  structural  the subject matter EXISTS and nothing produced the gate's input —
              a gap the repo CAN close (the axe-report case it was written for).

Gates 4, 6, 7 and 28 already called the identical situation `na` ("0
lib/Controller PHP file(s) in this diff"). The four touched by #258 were the
outliers; they now match. Audited every remaining `structural` call site
(24, 28x2, 30, 33) — each is a genuine gap inside a NON-empty scope, and all
five keep their category.

What #258 bought is unchanged, because it lives in the rendering and not the
accounting: an unopened scope prints NOT APPLICABLE, which is not PASS.

Measured, package-vs-package on unchanged trees (exit 98 -> exit 0, with the
EMITTED gate count identical on both sides — nothing was widened away, only
the denominator moved):

  doriath #191             98 -> 0   (57 green both sides)
  doriath development tip  98 -> 0   (53 green both sides)
  larpingapp  b0c60f734    98 -> 0   (53 green both sides)
  softwarecatalog 31d87d1  98 -> 0   (57 green both sides)

On doriath#191 the changed controller is judged by the ten gates whose subject
it actually is (5, 6, 7, 8, 9, 14, 16, 25, 47, 48). gate-63 is correctly `na`:
ADR-079 is about WHERE A SETTINGS SURFACE IS DECLARED, and every rule in
check_settings() reads a manifest or src/menu-layout.json. Making it judge
controller bodies was considered and rejected — check_store() records that the
same widening (walking lib/ while the manifest half honoured ADR-020)
"blocked EVERY manifest-touching PR in that repo, permanently".

TESTS. test_gate_empty_scope_never_passes.sh grows from three arms to five and
pins both directions:

  ARM 1  planted true positives still caught full-tree (anti-widening)
  ARM 2  empty scope is NOT APPLICABLE, never PASS, and does not exit 98
  ARM 3  a clean in-scope manifest still PASSes
  ARM 4  a GENUINELY structural gap (--axe-enabled, no report) STILL exits 98
  ARM 5  a real ADR-079 violation in an in-scope manifest still FAILs — `na`
         must not swallow a subject that IS in the diff

Mutation-checked, each mutation asserting `old in source` before rewriting so a
mutation that never applied cannot masquerade as a survivor:

  M1  revert gate-63 to `structural`        -> killed by ARM 2
  M2  gate-33's real structural gap -> `na` -> killed by ARM 4
  M3  gate-63 declares `na` unconditionally -> killed by ARM 5 ONLY

M3 is why ARM 5 exists: ARM 3 does not catch it, because a clean manifest still
returns rc=0 and passes. Also fixes _verdict(), which parsed "NOT APPLICABLE"
as "NOT" under a single-token pattern.

Full package suite: 41 helper suites + 59 entry-point tests, 0 failures; the 2
pre-existing quarantines are untouched.

ALSO: emit the gate package's own SHA into the run output. The fleet consumes
this package at @main UNPINNED, so two runs minutes apart can be two different
programs — doriath's development tip was gated green 8 seconds before the
classification change landed. This does not fix the strict-subset merge rule's
unsoundness; it makes it CHECKABLE, which is the prerequisite for any fix. The
policy question stays open in #268. Resolution order is
HYDRA_GATES_PKG_SHA -> git checkout of the package -> VERSION file -> a loud
UNKNOWN, never silence: a missing line is indistinguishable from an older
package that never had one.

Refs #268, #258, #242, #240, #169
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant