review round 1: record PR #209's state, and close a census blind spot

Review finding (P2): docs/active-work.md and docs/agent-handoff.md both
still said "PR not yet open". This file's own rule is that an open PR
gets a lane recording its state — "When a PR is opened, give it a lane",
written after #171 drifted 153 commits while invisible here. Both now
carry PR #209, its six commits, and the first CI run.

That run is worth recording rather than merely citing, because it
discharges the one risk the framing could not close locally. ALL 14
CHECKS GREEN, including both new jobs (Test (crdt) 12m20s, M10 Perf
Gates (crdt) 5m40s) and the macOS/luajit leg that is the documented
flake surface. This was the first time in the project's history that
any of these tests executed in CI.

ACCEPTANCE 8 HOLDS AGAINST THE REAL RUN. Test (crdt) reported 3,717
passed / 0 failed / 30 ignored: the 3,746 all-targets census (with
basedpyright NOT skipped, as it is locally) plus one doc test, less the
30 ignored. The job demonstrably compiled and ran the crdt corpus rather
than reporting green over nothing — which was the whole point of writing
that criterion while the local sweep was already green.

A trap for the next reader is recorded with it: DO NOT SUBTRACT THE TWO
JOBS' TOTALS. Test (ubuntu/luajit) reports 3,485 and Test (crdt) 3,747,
a difference of 262 rather than 279, because the jobs run different
SETS — the non-crdt job adds pmacs-protocol's 17 tests. The dark count
is the all-targets comparison, 3,746 - 3,467. The wrong number looks
entirely plausible.

Chasing that discrepancy found a real gap, and one this lane's own tool
could never have surfaced: PMACS-PROTOCOL HAS ITS OWN `crdt` FEATURE.
It gates no tests, so a per-test census reports 17 either way and scores
the crate as unaffected — but it changes `cfg!(feature = "crdt")`
EXPRESSIONS inside InstanceCapabilities::default and
FrontendCapabilities::default, so those same 17 tests exercise different
runtime values under it. CI had only ever run the non-crdt ones. Closed
with an explicit `-p pmacs-protocol --all-targets --features crdt` step,
verified 17/17.

The blind spot is structural, not an omission, so it is documented at
the tool rather than fixed in it: scripts/feature-census censuses the
workspace DEFAULT MEMBER, because that is what a bare
`cargo test --all-targets` builds. Sibling crates are invisible to it no
matter what configs are passed. The durable lesson: A FEATURE CAN MATTER
TO A CRATE A PER-TEST CENSUS SCORES AS UNAFFECTED. Check siblings by
hand.

Verified: ci.yml parses at 10 jobs, script syntax clean, fmt,
diff-check, and the new protocol step green locally.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Levi Neuwirth 2026-08-01 11:26:28 -04:00
parent 57abcd940e
commit 6519bc3461
No known key found for this signature in database
5 changed files with 84 additions and 10 deletions

View File

@ -340,6 +340,20 @@ jobs:
PMACS_REQUIRE_SETSID: "1"
PMACS_REQUIRE_BASH: "1"
- run: cargo test --doc --no-default-features --features luajit,crdt
# pmacs-protocol under `crdt`, which nothing else runs. Its own
# feature is NOT inert: it gates no tests, so a test census sees
# 17 either way, but it changes `cfg!(feature = "crdt")`
# EXPRESSIONS inside `InstanceCapabilities::default` and
# `FrontendCapabilities::default` — so the same 17 tests exercise
# different runtime values. CI had only ever run the non-crdt
# ones.
#
# This is also a blind spot of scripts/feature-census by
# construction: it lists the workspace DEFAULT MEMBER (`pmacs`),
# so no per-test census of the root package can ever surface a
# sibling crate's coverage. The `test` job carries the same
# explicit invocation for the same underlying reason.
- run: cargo test -p pmacs-protocol --all-targets --features crdt
acceptance:
name: M1 Acceptance Gates

View File

@ -148,14 +148,38 @@ form. All four steps ran clean. **The two-argument form still does not
work** for a remote-only branch (`fatal: invalid reference`), which is
why every lane below spells out the `-b` form.
## CI CRDT coverage lane — IMPLEMENTED on `ci-crdt-coverage`, PR not yet open
## CI CRDT coverage lane — **PR #209 OPEN, all 14 checks green**
**This lane had no branch and no owner from #166 until 2026-08-01.** It
now has both. Framing:
`docs/ci-crdt-coverage-framing.md` revision 3 (approved at revision 2).
now has both, and a PR. Framing:
`docs/ci-crdt-coverage-framing.md` revision 4 (approved at revision 2).
Branch `ci-crdt-coverage` off `githubsucks/main` @ `4223dd3`, developed
in the primary checkout on the laptop, not a worktree.
- **PR: <https://github.com/levineuwirth/pmacs/pull/209>**, opened
2026-08-01, awaiting user review. Six commits: `7a9cf5b` clippy,
`06abbac` m10-perf-gates, `7a8746d` crdt-test, `a776bc3`
feature-census, `57abcd9` docs, plus a review-round commit for the
pmacs-protocol gap below.
- **First CI run: all 14 checks green** (run `30705124856`), including
both new jobs — `Test (crdt)` 12m20s and `M10 Perf Gates (crdt)`
5m40s — and the macOS/luajit leg that is the usual flake surface.
**This was the first time in the project's history that any of these
tests executed in CI.**
- **Acceptance criterion 8 — the count reconciliation — HOLDS against
the real run.** `Test (crdt)` reported **3,717 passed / 0 failed / 30
ignored**. That is 3,746 (the `--all-targets` census, with
`basedpyright` not skipped as it is locally) plus 1 doc test, minus
the 30 ignored: 3,716 + 1 = 3,717. The job demonstrably compiled and
ran the crdt corpus rather than reporting green over nothing.
- **Do not compare the two jobs' raw totals.** `Test (ubuntu/luajit)`
reports 3,485 and `Test (crdt)` 3,747 — a difference of 262, not 279.
The jobs run different *sets*: the non-crdt job adds pmacs-protocol's
17 tests, and 3,467 + 1 + 17 = 3,485 against 3,746 + 1 = 3,747. The
dark count is the all-targets comparison, 3,746 3,467 = **279**.
A reviewer who subtracts the job totals gets a wrong number that looks
plausible.
- **Root cause, unchanged:** `.github/workflows/ci.yml` never enabled
the `crdt` feature anywhere. Every `#[cfg(feature = "crdt")]` test was
therefore **not compiled** in CI, not merely skipped — including the

View File

@ -94,11 +94,14 @@ anchor, so every item is startable.
#### Open lanes (branch exists, work not finished)
- **CRDT half of the corpus is dark in CI — LANE OPENED 2026-08-01**
(`ci-crdt-coverage`, `docs/ci-crdt-coverage-framing.md` rev 3;
implemented, PR not yet open). CI never enabled `crdt`, so those tests
were *not compiled*, not merely skipped — including 186 library tests
behind a **required** `CLAUDE.md` gate CI had never run.
- **CRDT half of the corpus is dark in CI — PR #209 OPEN, all 14 checks
green** (`ci-crdt-coverage`, `docs/ci-crdt-coverage-framing.md` rev 4;
opened 2026-08-01, awaiting user review). CI never enabled `crdt`, so
those tests were *not compiled*, not merely skipped — including 186
library tests behind a **required** `CLAUDE.md` gate CI had never run.
The first run reported **3,717 passed / 0 failed / 30 ignored** in
`Test (crdt)`, reconciling exactly to the census, so the job is
demonstrably not vacuous.
**Re-measure before quoting a number**, and there is now a tool:
`scripts/feature-census luajit luajit,crdt`. Measured 279 dark at
`4223dd3`; 275 recovered, 4 excluded with stated reasons. Two

View File

@ -1,7 +1,26 @@
# Framing — the CRDT half of the test corpus is dark in CI
**Revision 3.** Status: **implemented** on branch `ci-crdt-coverage`,
based on `githubsucks/main` @ `4223dd3` (#208). Approved at revision 2.
**Revision 4.** Status: **PR #209 open, all 14 checks green on the first
run.** Branch `ci-crdt-coverage`, based on `githubsucks/main` @
`4223dd3` (#208). Approved at revision 2.
**Revision 3 → 4** records the first CI run and one review finding:
- **Acceptance 8 holds against the real run.** `Test (crdt)` reported
**3,717 passed / 0 failed / 30 ignored** — 3,746 census (with
`basedpyright` not skipped as it is locally) + 1 doc test 30
ignored. The job compiled and ran the corpus; it did not report green
over nothing. §1.7's warning that the local green proved nothing about
hosted runners is now **discharged by evidence** rather than still
outstanding.
- **A gap `feature-census` structurally could not see**: pmacs-protocol
has its own `crdt` feature. It gates no tests — the census is 17
either way — but it changes `cfg!(feature = "crdt")` *expressions*
inside the capability defaults, so those 17 tests exercise different
runtime values under it, and CI had only ever run the non-crdt ones.
Closed by an explicit `-p pmacs-protocol --features crdt` step. **A
feature can matter to a crate a per-test census scores as
unaffected**, and the script's header now says so.
**Revision 2 → 3** records implementation findings, not a new design
round. Three things changed:

View File

@ -63,6 +63,20 @@
# what a fix delivers, so ignored tests are counted separately via a
# second `--list --ignored` pass.
#
# SCOPE LIMIT, and it is structural rather than an omission: this
# censuses the workspace DEFAULT MEMBER only (`pmacs`), because that is
# what a bare `cargo test --all-targets` builds. Sibling crates
# --- pmacs-protocol, pmacs-gpu --- are invisible to it no matter what
# configs are passed.
#
# That blind spot is not hypothetical. pmacs-protocol has its own
# `crdt` feature which gates NO tests, so a census of it would report
# zero dark either way --- yet the feature changes
# `cfg!(feature = "crdt")` expressions inside the capability defaults,
# so its 17 tests exercise different runtime values under it. A feature
# can therefore matter to a crate this script would score as
# unaffected. Check sibling crates by hand.
#
# CARGO_TERM_COLOR is pinned off so escape codes cannot break the
# anchored matches, the same precaution and for the same reason as
# scripts/bite.