Review finding (P2), and it is this lane's own defect class one level
down. Round 1 added a `-p pmacs-protocol --features crdt` step so that
crate compiles both ways. That step EXECUTED
`InstanceCapabilities::default` in both configurations and asserted
NOTHING about it: the crate's only use of that value is a transport
round-trip, and a round-trip is invariant to the values. An all-false
default — or one whose three fields disagreed with each other — encodes,
decodes, and passes identically in both builds.
Running code is not testing it. That is the same sentence this whole
lane is about, and round 1 committed the smaller version of it while
fixing the larger one.
Three tests now pin the defaults, and the split is the point:
* under `crdt`: multi_frontend, crdt_replica and semantic_render all
default true. Advertising false on a CRDT build would strand every
frontend in single-frontend mode.
* without `crdt`: all three default false. Advertising true would be
wire-protocol false advertising — those code paths are
conditionally compiled out.
* FrontendCapabilities::default is all-false in BOTH builds, and this
test is DELIBERATELY NOT feature-gated.
That third one pins an asymmetry nothing else did.
FrontendCapabilities derives Default and is feature-INVARIANT, while
InstanceCapabilities is feature-DEPENDENT. It is load-bearing rather
than an oversight: an instance advertises what it can do, a frontend
OPTS IN through the negotiation handshake, and a v1 frontend has no
local CRDT state regardless of how the crate it links was compiled.
Making the frontend side track the feature would have frontends
claiming support they do not have. A future edit that "makes them
consistent" now fails a test that says why not to.
All three fields are asserted separately rather than by comparing whole
structs, because they track one `cfg!` and a change flipping only some
of them is exactly the regression worth catching.
Bite-verified rather than assumed: mutating `multi_frontend` to a
literal false gives `FAILED. 18 passed; 1 failed` with the expected
assertion message; restoring returns 19/19. Both configurations now
report 19 tests, up from 17, with the correct cfg-gated test running in
each.
Left untested and recorded instead: `InstanceCapabilities::crdt_replica`
carries `#[serde(default = "default_true")]`, a THIRD default mechanism
that is unconditional and therefore disagrees with the `Default` impl in
a non-CRDT build. Exercising it needs a self-describing format and this
crate's only serde dependency is postcard, which is not one. Adding
serde_json as a dev-dependency to test a divergence this lane did not
introduce is scope creep.
Also corrects a note that round 1 made stale: the ledger's "do not
subtract the two jobs' totals" figures (3,485 and 3,747) were measured
BEFORE round 1 added the protocol step, and round 2 adds two tests to
that crate. Expected totals are now 3,487 and 3,766. The root-package
census is untouched at 3,467 / 3,746 — the new tests live in a sibling
crate, which is precisely the region scripts/feature-census cannot see.
Verified: fmt, diff-check, clippy on pmacs-protocol in both feature
configurations, and workspace clippy --features crdt --keep-going.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
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) <noreply@anthropic.com>
`.github/workflows/ci.yml` never enables the `crdt` feature anywhere, so
every `#[cfg(feature = "crdt")]` test is not compiled in CI — not
skipped, not filtered, not reported. 279 tests have never executed in
CI, and 186 of them are in the library, whose `cargo test --lib
--features crdt` invocation CLAUDE.md lists as a required pre-PR gate.
CI has never once run a required gate.
Named in docs/active-work.md as "NEEDS A LANE" since it was found while
gating #166, with no branch and no owner until now.
Revision 2, approved. What the scout established beyond the ledger:
- The census moved and was re-measured at 4223dd3: 279 dark, not the
ledger's 273. Plus a fact the old census did not carry — eight test
binaries contain zero tests under CI flags, so they build, run, and
report ok with nothing in them.
- "279 dark" overstates the fix. Eleven are #[ignore]d, so a plain crdt
leg recovers 268. All 279 are now assigned a disposition: 268 by the
leg, 7 by a new m10-perf-gates job, 3 deliberately manual, 1 an
ignored known-defect marker. The lane recovers 275 of 279 and the
other four are excluded for stated reasons rather than left as
residue.
- The classification docs/active-work.md called "the lane's first task"
is finished, and it found three dispositions rather than two:
benches awaiting a job, deliberately-manual operator tests, and
known-defect markers.
- The ledger's clippy inventory was stale in both directions. It warned
its own list was "a lower bound, not an inventory" because clippy
aborts on first failure; --keep-going is what converts it. The
complete set is eight findings across four files, none behavioral.
- A hole in the proposed fix shape: the gpu-render job runs
`cargo test -p pmacs-gpu`, a different package from the four root-package
suites the fix wants to move there. And PMACS_REQUIRE_GPU is absent
from two of those four, so it cannot serve as blanket proof they ran.
- The full serialized crdt sweep is green: 3,715 passed, 0 failed, 30
ignored, reconciling exactly to the 3,746 census. That green result
shaped the acceptance criteria more than a red one would have — a
green pre-measurement is the condition under which a vacuous CI job
ships unnoticed, so the criteria gained a count reconciliation and a
deliberately-broken-test bite.
Revision 1 -> 2 also corrects revision 1's classification of
m10_10_perf, which was wrong in a way that would have made the lane
worse: the suite is a deliberate CI-default regression tripwire with
generous bounds, not a bench, so adding #[ignore] would have shipped a
coverage reduction inside a coverage lane.
Framing only. No workflow change, no source change, no branch plan
executed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Levi Neuwirth