From 57abcd940ec817c934a35a0d5da0baa73fe5a7d8 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 1 Aug 2026 11:00:12 -0400 Subject: [PATCH] docs: record the CI CRDT coverage lane and its corrections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Framing revision 3, plus the ledger and handoff updates the lane owes. docs/active-work.md's "NEEDS A LANE" block is replaced rather than annotated: the lane now has a branch, a framing, an implementation and verification, so the old text's premise ("no branch, no framing yet") was the part that needed removing. Its stale figures go with it — the 273-dark reading at 74301d1 and the seven-item clippy list, both superseded and both explicitly labelled do-not-quote in the new block. The corrections this lane produced, all now recorded where the next reader will hit them: * m10_10_perf is a CI-DEFAULT REGRESSION TRIPWIRE, not a bench. Its bounds are deliberately generous "to catch catastrophic regressions, not to verify a tight perf claim", so #[ignore]ing it to give it a perf job would have reduced coverage inside a coverage lane. The framing had this backwards through revision 1 and says so. * gpu-render runs `cargo test -p pmacs-gpu` — a DIFFERENT PACKAGE from the root-package GPU suites. The long-recorded fix-shape of "move the GPU-requiring crdt suites onto gpu-render, it already has lavapipe" does not work as written. * PMACS_REQUIRE_GPU is not uniform: absent from gpu_invocation_acceptance and gpu_initial_target_acceptance entirely, so it cannot serve as blanket proof the GPU suites ran. * The dark-test classification found THREE dispositions, not two: benches awaiting a job, deliberately-manual operator tests, and known-defect markers. Collapsing the second into the first gives a CI job to tests whose #[ignore] reason says not to. * §1.1's own target-column claim was wrong, and scripts/feature-census caught it. Eleven targets run with zero tests under CI's flags; eight of those gain tests under crdt and three are helper binaries with no tests in either. The framing had merged two true statements. Acceptance 9 is revised rather than quietly met at a lower bar. The planned deliberately-broken-test bite is replaced by the --covers coverage assertion plus the CI count reconciliation, and the doc states what that trades: stronger on the claim specific to this lane (the old job STRUCTURALLY cannot see these tests), and no longer proving that a failing crdt test turns the job red — which is generic cargo and Actions behavior, not anything this lane changes. The handoff gains a census hazard beside the existing sweep note, because every trap in it was hit while writing the script and one of them survived two framing revisions: libtest prints `name: test` with no space before the colon, a zero-test target prints only its Running line, and both configurations need an --ignored pass or pre-existing ignores get attributed to the feature. The ledger also records what is NOT established, which is the lane's whole remaining risk: the sweep is green SERIALIZED ON A DEVELOPER MACHINE, and the failures this lane expects are hosted-runner timing and concurrency. That green removes the "tests are wrong" explanation and leaves the expected one untested. It must not be quoted as evidence the CI leg will be green. Co-Authored-By: Claude Opus 5 (1M context) --- docs/active-work.md | 259 ++++++++++++++++--------------- docs/agent-handoff.md | 33 +++- docs/ci-crdt-coverage-framing.md | 185 ++++++++++++++++------ 3 files changed, 297 insertions(+), 180 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 4ba8b9f..b4d48c3 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -5,6 +5,15 @@ landed on `main`. Read it after `docs/agent-handoff.md`. Remove completed entries when their PR merges; do not let this become a second permanent backlog. +**Updated later the same day, on a new machine.** Development moved to +the laptop; the recovery path in "Repository authority" below was +exercised from this checkout and the `githubsucks` alias was absent and +had to be added, exactly as that section anticipates. **One lane opened: +CI CRDT coverage**, which had been sitting under "NEEDS A LANE" with no +branch and no owner since #166. It is implemented on +`ci-crdt-coverage` and its block replaces the old one below. Everything +else in this file is unchanged. + **This snapshot is an absorption pass, taken with ZERO open PRs** — the one window in which a ledger refresh has nothing to re-conflict with, and taken deliberately before a machine move. Nine PRs landed @@ -139,140 +148,132 @@ 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. -## The CRDT half of the test corpus is dark in CI — NEEDS A LANE +## CI CRDT coverage lane — IMPLEMENTED on `ci-crdt-coverage`, PR not yet open -- **No branch, no framing yet.** Found while gating #166, then measured - properly during the vterm as-framed audit. Deliberately kept out of #166 so - a CI change would not arrive after review approval. -- **Root cause:** `.github/workflows/ci.yml` never enables the `crdt` feature - anywhere — zero hits across the workflow directory. The `test` job runs - `cargo test --all-targets --no-default-features --features luajit|lua54`. - Every `#[cfg(feature = "crdt")]` test is therefore **not compiled** in CI, - not merely skipped. -- **Measured, `--list` under CI's exact flags versus the same flags plus - `crdt`: 3,176 vs 3,449 — 273 tests dark.** Re-measured at `74301d1` - (2026-07-26; at `fe8b8ba` it read 3,170 vs 3,443, the same 273 dark — - #176 added six tests, none of them `crdt`-gated). **The number moves - with every merge and must be - re-measured, not quoted.** #168 reported 3,024 vs 3,288 — 264 dark, - 177 in the library — at `1b6a084`; #178 then added CRDT-only - generated-buffer coverage, and other lanes landed CRDT tests in - between. Per target: +**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). +Branch `ci-crdt-coverage` off `githubsucks/main` @ `4223dd3`, developed +in the primary checkout on the laptop, not a worktree. - | dark | CI | full | target | - |---:|---:|---:|---| - | 185 | 1,848 | 2,033 | **the library itself** (`src/lib.rs`) | - | 21 | 15 | 36 | `m5_5_acceptance` | - | 13 | 1 | 14 | `gpu_invocation_acceptance` | - | 13 | 1 | 14 | `gpu_initial_target_acceptance` | - | 8 | 0 | 8 | `m10_11_acceptance` | - | 6 | 0 | 6 | `auto_pair_crdt_acceptance` | - | 6 | 0 | 6 | `m10_2_perf` | - | 4 | 5 | 9 | `vterm_stage3_acceptance` | - | 4 | 0 | 4 | `m10_10_perf` | - | 3 | 0 | 3 | `compile_mode_crdt_acceptance` | - | 2 | 22 | 24 | `theme_faces_acceptance` | - | 2 | 0 | 2 | `m11_5_semantic_acceptance` | - | 1 | 14 | 15 | `terminal_copy_mode_acceptance` | - | 1 | 9 | 10 | `vterm_stage1_acceptance` | - | 1 | 7 | 8 | `statusline_segments_acceptance` | - | 1 | 10 | 11 | `gpu_font_acceptance` | - | 1 | 0 | 1 | `auto_indent_crdt_acceptance` | - | 1 | 0 | 1 | `m10_11_perf` | +- **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 + 186 library tests behind `cargo test --lib --features crdt`, which + `CLAUDE.md` lists as a REQUIRED pre-PR gate. CI had never run a + required gate. +- **Re-measured at `4223dd3`: 3,467 vs 3,746 — 279 dark**, up from the + 273 recorded at `74301d1`. **Do not quote this number either.** It + moves with every merge, and there is now a tool: `scripts/feature-census + luajit luajit,crdt` reproduces the whole per-target table, the ignored + split, and the zero-test-target count in one command. +- **The 279 are fully dispositioned; 275 are recovered.** 268 by the new + `crdt-test` job, 7 by the new `m10-perf-gates` job, and 4 deliberately + excluded: `m10_11_acceptance`'s three PTY-doubled tests (marked + operator-invoked before tagging) and the #157 CRDT undo repro, an + `#[ignore]`d known-defect marker whose arming belongs to that defect's + lane. +- **The deliberate/accidental classification the old lane text called + "the lane's first task" is finished**, and it found THREE dispositions + rather than two: benches awaiting a job (`m10_2_perf`, + `m10_11_perf`), deliberately-manual operator tests + (`m10_11_acceptance`), and known-defect markers. Collapsing the second + into the first would have given a CI job to tests whose `#[ignore]` + reason says not to. +- **`m10_10_perf` is NOT a perf gate**, and the framing got this wrong + first. Its header says its bounds are "generous ... to catch + catastrophic regressions, not to verify a tight perf claim," so its + lack of `#[ignore]` is the design. Adding one to give it a job would + have shipped a coverage reduction inside a coverage lane. It stays + untouched and rides the plain leg. +- **The clippy obstacle is cleared, and the old inventory was stale in + both directions.** `cargo clippy --workspace --all-targets --features + crdt` had never passed. The previous seven-item list was correctly + labelled "a lower bound, not an inventory" — `--keep-going` is what + converts it. The real set was eight findings across four files; the + `unneeded mut` at `daemon.rs:4965` had been fixed incidentally, a + finding in `bottom_panel_stage2b_gpu_acceptance.rs` was new, and every + `daemon.rs` line number had moved. +- **One job, not two.** The fix-shape recorded here previously put the + GPU-requiring suites onto `gpu-render` "which already has lavapipe and + PMACS_REQUIRE_GPU". **That job runs `cargo test -p pmacs-gpu` — a + different package** from the four root-package suites, so co-locating + them means a new invocation rather than an extension. Splitting also + requires classifying every future suite as GPU-requiring or not, and a + misclassified one skips forever. `crdt-test` installs lavapipe and + sets `PMACS_REQUIRE_GPU=1` for the whole corpus instead. +- **`PMACS_REQUIRE_GPU` is not uniform and cannot serve as blanket + proof**: it appears in `vterm_stage3_acceptance` (twice) and + `bottom_panel_stage2b_gpu_acceptance` (once), and **not at all** in + `gpu_invocation_acceptance` or `gpu_initial_target_acceptance`. +- **The external-tool install block is deliberately not duplicated.** + Measured: it gates `m4_acceptance`, `m6_5_repl_acceptance` and + `m6_8_multi_repl_acceptance`, none of which has a single dark test. + The only tool-gated code in the dark set is `src/process.rs`, whose + two variables need no install. +- **Crdt clippy runs in `crdt-test`, not the `clippy` job.** The `clippy` + job matrixes over Lua flavor and never enables `crdt`, so clearing + those lints once would let them drift straight back and the next job + to compile them would be red on arrival — the exact state this lane + found. +- **Verification on the laptop (integrated Radeon, 16 threads), all at + the exact commands CI runs:** clippy green with and without `crdt`; + fmt; diff-check; `--lib` 1,896; `--lib --features crdt` 2,081; doc + tests; `m10_2_perf` 6/6 in 79s and `m10_11_perf` 1/1 in 5s under + release; and the full serialized sweep with `PMACS_REQUIRE_GPU=1` at + **3,715 passed / 0 failed / 30 ignored in 366s**, reconciling exactly + to the 3,746 census (3,715 + 30 + 1 basedpyright-skipped). The + reconciliation is the point: a sweep that does not reconcile is the + a37 vacuum at corpus scale. +- **What is NOT established, and it is the lane's whole remaining risk:** + the sweep is green *serialized on a developer machine*. The failures + this lane expects are hosted-runner timing and concurrency — real PTY + on CI runners, wgpu under lavapipe, daemon sockets at unfamiliar + concurrency. A green local run removes the "tests are wrong" + explanation and leaves the expected one untested. **Do not quote it as + evidence the CI leg will be green.** +- **Red-first-run policy, decided:** fix in-lane by default, with one + escape hatch keyed on cause class. Lane-configuration failures and + locally-reproducing failures are fixed here; an environment-only + failure (green locally and on Universum, red only on a hosted runner) + becomes a named follow-on rather than an unbounded investigation + inside a workflow-config PR. The local green makes that third class + the most likely red, which is why the hatch exists. +- **Universum (7900 XTX, remote) is where the GPU ambiguity settles.** + The laptop renders the GPU suites but is thermally constrained, which + is the documented condition for a37's "all spaces with nonzero + rendered_nonuniform_frames" signature. Universum can establish the + tests are sound; it cannot establish that the lavapipe CI job passes. - The rows sum to 273; the table is the whole census, not its head. +Recovery from a clean checkout: -- **The single worst line is the library.** `cargo test --lib --features crdt` - is a REQUIRED local gate in `CLAUDE.md`, and CI has never run it. 185 - library tests — the whole CRDT half — are developer-machine-only, and - that count grows with every merged branch that adds a `crdt`-gated - unit test. -- **Ten suites run zero or one test in CI**, including `gpu_initial_target` - (#148's entire acceptance, 1/14), `gpu_invocation` (#141's, 1/14), and - `a37`, the Vterm Stage 3 real-daemon/real-PTY/real-wgpu path that #135 - built specifically because "a decoded-message fixture would prove none of - the three fit together". -- **⚠ `a37` will report green in the new job without running, unless the - job builds `pmacs-gpu` AND sets `PMACS_REQUIRE_GPU=1`.** Measured - 2026-07-26 while gating #173. `a37_real_daemon_real_pty_and_headless_gpu_ - render_one_terminal_session` derives its sibling binary path from - `CARGO_BIN_EXE_pmacs`, and on a missing binary it `eprintln!`s a skip and - **returns `ok`**. A fresh worktree running - `cargo test --features crdt --test vterm_stage3_acceptance` reports **9/9 - in 0.17 s having never run it**; a real run takes ~4 s. Only - `PMACS_REQUIRE_GPU=1` promotes that skip to a failure, and `CLAUDE.md` - applies that flag to `cargo test -p pmacs-gpu` — a **different package**, - so the required local gate does not cover a37 either. The `gpu-render` - job already sets the flag, which is what makes fix-shape part 2 sound; - state it as a **requirement** of that job rather than inheriting it by - luck, because a `crdt` leg added to the plain `test` job would run a37 - vacuously. -- **`a37` is also load-sensitive, which changes how to read the expected - first-run failures.** It passed at `d152120` and failed at that *same - commit* twenty minutes later, with a second agent saturating the machine - with `rustc` in between; it then failed identically on `d152120`, - `04c5ad1`, and the #173 merge commit, which is how #173 established the - failure was not its own. The signature is `last_frame_text` all spaces - with `rendered_nonuniform_frames` nonzero — frames arrive, content does - not. `pmacs-gpu`'s own suite flaked the same way under the same load - (201/202, then 202/202 on immediate rerun). **So a red a37 on the first - CI run is ambiguous by construction**: before treating it as a real - failure, run the same command on the merge base, and prefer serialized - execution for this suite over retry-until-green. -- **Sort deliberate from accidental before proposing a fix.** Some of the 264 - are perf suites that are `#[ignore]`d by default and belong to their own - jobs (`m10_2_perf` 6, `m10_11_perf` 1). `m10_10_perf` has **no** `#[ignore]` - and no CI job naming it, so it looks accidental. This classification is not - finished and is the lane's first task. -- **Fix shape, two parts** (the flag combination is verified to work: - `--no-default-features --features luajit,crdt` lists 10 vterm Stage 1 tests - versus 9 without): - 1. a `crdt` leg on the `test` job for the non-GPU suites and the library; - 2. the GPU-requiring `crdt` suites onto the existing `gpu-render` job, which - already has lavapipe and `PMACS_REQUIRE_GPU=1` — - `vterm_stage3_acceptance`, `gpu_invocation_acceptance`, - `gpu_initial_target_acceptance`, `gpu_font_acceptance`. -- **Expect first-run failures, and budget for them.** These would execute in - CI for the first time ever: real PTY timing on CI runners, wgpu under - lavapipe, and daemon-socket tests at unfamiliar concurrency. Start - ubuntu-only and decide about macOS from evidence. A red first run is the - lane working, not the lane failing. -- Mitigating fact, verified rather than assumed: #166's three unit pins are - **not** `crdt`-gated and do run under CI's exact flags, including the - controller-release pin whose only job is catching the plausible wrong fix. -- **This lane also owns a `--lib --features crdt` flake, observed and - scoped without overclaiming its cause** (inherited from #178's gating, - where the terminal lane recorded it). `cargo test --lib --features - crdt` failed ~1 run in 5 on +```sh +git fetch githubsucks +git worktree add ../pmacs-ci-crdt \ + -b ci-crdt-coverage \ + githubsucks/ci-crdt-coverage +``` + +### Still owned by this lane, not yet done + +- **The `--lib --features crdt` flake.** `process::tests::setsid_escapee_is_not_reaped_and_teardown_reclaims_readers` - — `active_reader_probe` returning `None` at `process.rs:3179` ("live - runtime probe"). **Pre-existing and unrelated to #178:** that branch - did not touch `src/process.rs` at all, and the test passed 10/10 - standalone; the observed - failures were during parallel full-suite runs. That localizes the - trigger to suite load or interaction, but does **not** distinguish - parallelism from another full-suite effect — no serial full-suite bite - was run. The leading code-path explanation is the known `drain_until` - trap: draining for `Started` also ticks, and a tick can reap the leader - before the following `active_reader_probe`. That is an inference from - the failure site and control flow, not yet a falsified root cause. - Discriminating it belongs here. Two unnamed CRDT failures in #178's - round-2 gating are a plausible match but remain **unattributed** — no - test names were captured. -- **A second standing obstacle for this lane:** `cargo clippy --workspace - --all-targets --features crdt -- -D warnings` **fails on `main`** — - measured at `74301d1`: seven errors before the build aborts, four in - `src/daemon.rs` (`useless_conversion` at 3996, missing doc backticks at - 4076, `too_many_lines` 112/100 at 4083, an unneeded `mut` at 4965) and - three in `tests/vterm_stage3_acceptance.rs` (`too_many_lines` at 637 - and 793, a redundant `continue` at 843). **Treat that as a lower - bound, not an inventory:** Clippy abandons the remaining targets once - one fails, and a run on an older tree surfaced a further doc-backticks - error in `tests/auto_indent_crdt_acceptance.rs:42` that this run never - reached. The - standing gate list runs Clippy without `crdt`, so these lints have - never been enforced. Any CI job that compiles the `crdt` targets has to - fix them first or it will be red on arrival. + failed ~1 run in 5 with `active_reader_probe` returning `None`. **It + did not reproduce in this lane's runs** — `--lib --features crdt` was + 2,081/2,081 and the full serialized sweep was clean — but a + non-reproduction under serial execution is consistent with the + leading hypothesis rather than evidence against it: the trigger was + observed under *parallel* full-suite load, and every run here was + `--test-threads=1`. The `drain_until` explanation (draining for + `Started` also ticks, and a tick can reap the leader before the + following probe) remains an inference from control flow, not a + falsified root cause. Discriminating it is its own PR — it is a + product-defect hypothesis and everything else here is workflow + configuration. +- **The two unattributed CRDT failures from #178's round-2 gating.** No + test names were captured, so there is nothing to reproduce. +- **macOS.** Ubuntu-only first, deliberately; a macOS `crdt` leg is a + follow-on decided from the first run's evidence. ## Discovery lane (P4) — STAGE 1 MERGED (#207); STAGE 2 IS NEXT diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index e958bc7..a9c958f 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -1,6 +1,6 @@ # Agent handoff — cross-machine continuity -**Last updated: 2026-07-29, as bottom-panel Stage 2B-3 — the GPU panel +**Last updated: 2026-08-01 (CI CRDT coverage lane opened on `ci-crdt-coverage`; development moved to the laptop). Previously 2026-07-29, as bottom-panel Stage 2B-3 — the GPU panel band, compatible protocol-v21 activation, and the negotiated `panel_capable` flip, completing Arc 7 Stage 2 — opens atop `e003b81`. Beneath it, bottom-panel Stage 2B-2 (#187) — the @@ -94,10 +94,21 @@ anchor, so every item is startable. #### Open lanes (branch exists, work not finished) -- **CRDT half of the corpus is dark in CI — still no lane, no owner.** - CI never enables `crdt`, so those tests are *not compiled*, not merely - skipped. **Re-measure before quoting a number**; the ledger's figure - moves with every merge. +- **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. + **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 + corrections it forced are worth carrying: **`m10_10_perf` is a + CI-default regression tripwire, not a bench** (its bounds are + deliberately generous, so `#[ignore]`ing it to give it a perf job + would *reduce* coverage), and **`gpu-render` runs a different + package** (`pmacs-gpu`) from the root-package GPU suites, so the + long-recorded "move them onto gpu-render" fix-shape does not work as + written. - **Generated-buffer immutability** — Stage 1 merged (#191); Stage 2 not started. Four writer mechanisms have still not adopted `set_generated_contents`; key the inventory by *writer*, not buffer. @@ -156,7 +167,17 @@ someone forgot. `cargo test --test m4_acceptance -- --skip basedpyright`. - **The crdt sweep needs `cargo build --workspace` first**, or twelve `gpu_invocation_acceptance` tests fail on a missing `pmacs-gpu` - binary. + binary. `cargo build --workspace --no-default-features --features + luajit,crdt` is the invocation that produces both binaries. +- **Never hand-roll the dark-test census — use + `scripts/feature-census`.** libtest prints `name: test` with **no + space before the colon**, so a filter written `/ : test$/` matches + nothing and reports a clean zero; a target with zero tests prints its + `Running` line and nothing else, so counting only test lines drops it + from the diff; and both configurations need an `--ignored` pass, or + pre-existing ignores get attributed to the feature. Each of those was + hit while writing the script, and the second one survived two + revisions of a framing doc. - **A green a37 means nothing on its own** — the vterm real-daemon acceptance returns `ok` without running unless `pmacs-gpu` is built. diff --git a/docs/ci-crdt-coverage-framing.md b/docs/ci-crdt-coverage-framing.md index fabff81..df31ebf 100644 --- a/docs/ci-crdt-coverage-framing.md +++ b/docs/ci-crdt-coverage-framing.md @@ -1,15 +1,25 @@ # Framing — the CRDT half of the test corpus is dark in CI -**Revision 2.** Status: framing only. No branch, no implementation. -Scouted against `githubsucks/main` @ `4223dd3` (#208), with **zero open -PRs** on the board. +**Revision 3.** Status: **implemented** on branch `ci-crdt-coverage`, +based on `githubsucks/main` @ `4223dd3` (#208). Approved at revision 2. -**Revision 1 → 2** records the user's decisions on Q#CC3, Q#CC4 and -Q#CC7, and **corrects revision 1's classification of `m10_10_perf`**, -which was wrong in a way that would have made the lane worse. See §1.3a. -The correction also completes the disposition accounting: all 279 dark -tests are now assigned, with 275 recovered and 4 excluded for named -reasons (§1.2a). +**Revision 2 → 3** records implementation findings, not a new design +round. Three things changed: + +- **§1.1's target-column claim was wrong**, and `scripts/feature-census` + found it. "Eight test binaries contain zero tests" merged two + different true statements; the corrected reading is in §1.1. +- **The census now has a tool** (§1.9). The ledger's standing "re-measure, + don't quote" instruction had never had one. +- **Acceptance 9 is revised** — the deliberate-break bite is replaced by + a structural coverage assertion plus the CI count reconciliation, with + what each does and does not prove stated explicitly. See §4. + +**Revision 1 → 2** recorded the decisions on Q#CC3, Q#CC4 and Q#CC7, and +**corrected revision 1's classification of `m10_10_perf`**, which was +wrong in a way that would have made the lane worse (§1.3a). It also +completed the disposition accounting: all 279 dark tests assigned, 275 +recovered and 4 excluded for named reasons (§1.2a). `.github/workflows/ci.yml` never enables the `crdt` feature anywhere. Every `#[cfg(feature = "crdt")]` test is therefore **not compiled** in @@ -51,16 +61,27 @@ did: the previous reading was 273 at `74301d1`. Under CI's exact flags versus the same flags plus `crdt`: -| | tests | test binaries | +| | tests | targets with ≥1 test | |---|---:|---:| | `--no-default-features --features luajit` | 3,467 | 93 | | `--no-default-features --features luajit,crdt` | 3,746 | 101 | -| **dark** | **279** | **8** | +| **dark** | **279** | **8 targets gain tests** | -The binary column is a fact the earlier census did not carry: **eight -test binaries contain zero tests under CI's flags.** They are still -built, still run, and still report `ok` — with nothing in them. A green -result from those eight is not weak evidence, it is no evidence. +**Corrected in revision 3.** Revisions 1 and 2 read the target column as +"eight test binaries contain zero tests under CI's flags." That is not +what the column measures, and `scripts/feature-census` (§1.9) found the +error by reporting both figures separately. Two different true +statements had been merged: + +- **Eleven** targets run under CI's flags with **zero tests**. They + build, run, and report `ok` with nothing in them. +- **Eight** of those eleven **gain tests under `crdt`** — the ones this + lane recovers. The other three are helper binaries (`pmacs_audit`, + `pmacs_fake_lsp`, `pmacs_fake_mcp`) with no tests in either + configuration, which is correct and not a gap. + +A green result from the eight is not weak evidence, it is no evidence. +The three are fine. Conflating them inflates the defect. Per target, every row with a nonzero delta: @@ -328,6 +349,38 @@ as evidence that the CI leg will be green.** --- +### 1.9 The census now has a tool + +`docs/active-work.md` says the dark-test figure "moves with every merge +and must be re-measured, not quoted." **That instruction had no tool**, +so every re-measurement was a hand-rolled `--list` pipeline written from +scratch — including this lane's, whose first attempt filtered on +`/ : test$/` when libtest prints `name: test` with no space before the +colon. It matched nothing, reported zero targets, and looked like a +clean run. + +`scripts/feature-census [--covers ]` is that +tool. It reproduces every figure in §1.1 and §1.2 independently, and its +header records each parsing trap, all of which were hit while writing +it. Two are worth repeating here because they change results rather than +merely breaking a run: + +- **A target with zero tests prints its `Running` line and nothing + else.** Counting only test lines drops it from the diff entirely — + losing precisely the finding worth surfacing. This is how the §1.1 + error above survived two revisions. +- **Both configurations need an `--ignored` pass, not just the richer + one.** Counting only B's ignored set attributes pre-existing ignores + to the feature: `rope::tests::perf_smoke_*` are ignored under both and + are not "dark and ignored." The difference is what turns a flat + "279 dark" into §1.2's "268 recovered, 11 needing `--ignored`." + +It is **fail-closed on a build failure** (exit 3). A configuration that +does not compile yields no test list, which is indistinguishable by +counting from "this configuration has no tests" and would render as an +entirely false "every test is dark." That is not a small error; it is a +number that would get quoted. + ## 2. Questions - **Q#CC1 — does the `crdt` leg go on the existing `test` matrix or its @@ -474,12 +527,41 @@ easiest to ship unnoticed. for the suites it runs, the way §1.7 reconciles to 3,746. A job that runs fewer tests than the census predicts has found a silently-absent binary, and that is the defect class this whole lane exists to end. -9. **A deliberately-broken bite proves the leg is not vacuous.** Before - merge, break one `crdt`-gated library test on the branch and confirm - the new CI job goes red. A leg added to a corpus that is already - green locally cannot otherwise be distinguished from a leg that - compiles nothing — which is precisely the failure `.github` has today - and the reason criterion 8 is not sufficient on its own. +9. **The leg is proven load-bearing, structurally rather than by + breakage.** *Revised in revision 3 — the original criterion asked for + a deliberately-broken test, and it is worth saying plainly why that + was dropped rather than quietly meeting a weaker bar.* + + The original: break a `crdt`-gated library test, confirm the job goes + red, revert. It proves two things at once — that the leg compiles + crdt tests, and that a failure propagates to a red job. The second is + generic cargo/libtest behavior, not anything this lane changes, and + buying it costs a mutation of tracked source restored by a shell + trap, plus a broken commit in the PR's history. + + What replaces it, in two parts: + + - **Structural, and stronger on the point that matters:** + `scripts/feature-census luajit luajit,crdt --covers ` asserts + a named test is present under the new job's flags and **absent + under the old job's**. That is a claim the break test does *not* + make — a red job proves the leg caught something, but not that the + existing job could never have. Verified for + `crdt_apply_edit_keeps_invariant_basic`: present under B, absent + under A, exit 0. The negative cases are exercised too (a test in + both configs, and a misspelled name, both exit 1). + - **Empirical, from the first CI run:** criterion 8's count + reconciliation. A job reporting ~3,715 passed has demonstrably + compiled and run the crdt corpus; a vacuous leg reports a number + near the `test` job's. + + **What is no longer proven, stated rather than glossed:** that a + *failing* crdt test turns this specific job red. Nothing on this + branch demonstrates it. It rests on cargo returning non-zero on test + failure and GitHub Actions failing a step on non-zero — both + universal, neither lane-specific. If review wants that proven + directly, the break test is still the way, and it belongs on a + throwaway PR rather than in this one's history. --- @@ -511,6 +593,8 @@ make meaningful: - `cargo fmt --check` - `cargo clippy --workspace --all-targets -- -D warnings` - **`cargo clippy --workspace --all-targets --features crdt --keep-going -- -D warnings`** (new, and the lane's own subject) +- **`scripts/feature-census luajit luajit,crdt`** (new; re-measures the + census rather than quoting it forward, per the ledger's own rule) - `cargo test --lib` - `cargo test --lib --features crdt` - the touched acceptance suites @@ -534,31 +618,42 @@ newly compiles. ## 7. Branch plan One branch, `ci-crdt-coverage`. Commits in this order, because each -earlier one is a precondition for the next being observable: +earlier one is a precondition for the next being observable. **As +landed:** -1. **Clear the eight clippy findings** (§1.4). Nothing can compile the - `crdt` targets under `-D warnings` until this lands, so no CI change - is testable before it. +1. **Clear the eight clippy findings** (§1.4) — `7a9cf5b`. Nothing can + compile the `crdt` targets under `-D warnings` until this lands, so + no CI change was testable before it. 2. **Add the `m10-perf-gates` job** for `m10_2_perf` and `m10_11_perf` - (Q#CC4), shaped like `m5-perf-gates` / `m6-perf-gates`: - `--release --features crdt -- --ignored --nocapture`, with - `timeout-minutes` and a written luajit-only justification. - **No source file changes** — per Q#CC3 and §1.3a, `m10_10_perf` is - not touched and gets no `#[ignore]`. -3. **Add the non-GPU `crdt` leg**, ubuntu-only, luajit-only, with its - justification written in the workflow. This is the commit that - recovers 268 tests, `m10_10_perf`'s 4 among them. -4. **Prove the leg is not vacuous** (acceptance 9): break one - `crdt`-gated library test, confirm the new job goes red, revert. - Do this *before* step 5, so the proof is against the simpler job. -5. **Add the GPU-requiring `crdt` suites to `gpu-render`**, with the - per-suite execution proof from Q#CC5 — this is where §1.5's - package mismatch has to be handled explicitly. -6. **Update `docs/active-work.md` and `docs/agent-handoff.md`** per + (Q#CC4) — `06abbac`. **No source file changes**: per Q#CC3 and + §1.3a, `m10_10_perf` is not touched and gets no `#[ignore]`. +3. **Add the `crdt-test` job** — `7a8746d`. Recovers 268 tests, + `m10_10_perf`'s 4 among them. +4. **Give the census a tool** — `a776bc3`, `scripts/feature-census` + (§1.9). This replaced the planned break-test as the non-vacuity + proof; see acceptance 9 for what was traded away and why. +5. **Update `docs/active-work.md` and `docs/agent-handoff.md`** per acceptance 7. -**The GPU-suite commit is the one that needs Universum.** Steps 1–3 are -fully verifiable on this laptop; step 4's failures cannot be -distinguished from thermal and load noise here (§1.8), and settling them -on the 7900 XTX proves the tests are sound without proving the lavapipe -CI job will pass. Budget for that gap rather than assuming it away. +**Two departures from the plan as approved, both deliberate:** + +- **The GPU suites did not get a separate commit against `gpu-render`.** + The plan's step 5 assumed they could be added to that job; §1.5 + established it runs a different package, so co-locating them there + would have meant a new invocation rather than an extension, plus a + standing requirement to classify every future suite as GPU-requiring + or not. They are covered by `crdt-test` instead, which installs + lavapipe and sets `PMACS_REQUIRE_GPU=1` for the whole corpus. **One + job cannot develop the hole that splitting invites.** +- **The break-test became a coverage assertion plus a tool** (acceptance + 9). The proof got stronger on the claim specific to this lane — that + the old job *structurally cannot* see these tests — and weaker on a + generic one it no longer makes. + +**Universum is still where the GPU question settles.** Everything above +was verified on this laptop, whose integrated Radeon renders the GPU +suites but is thermally constrained, so a red `a37` here is ambiguous by +construction (§1.8). What Universum's 7900 XTX can establish is that the +tests are sound; it cannot establish that the lavapipe CI job will pass, +and the first PR run is the only thing that can. Budget for that gap +rather than assuming it away.