From 6b4d52a5bc34b735d53bc36b556db0314b8a250a Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 4 Aug 2026 12:08:28 +0200 Subject: [PATCH] review round 1: the crdt suite Stage 3 missed, and two stale explanations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1 — compile_mode_crdt_acceptance was never revised. Its three `compile.run` fixtures still omitted `display`, so the flip placed output in the panel with `select = false`, the source buffer stayed active, and `adopt_next_buffer` timed out waiting for a snapshot that only arrives via the ACTIVE-BUFFER-FOLLOW path. Its subject is CRDT convergence of a generated buffer, not placement, so it takes the explicit opt-out — and the reason is recorded at `adopt_next_buffer`, the helper that actually depends on it, rather than at each call site. WHY MY OWN VERIFICATION MISSED IT, which matters more than the fix: the Stage 3 census and every sweep I ran used `--features luajit` WITHOUT `crdt`, so no crdt-gated suite was ever exercised. The census was therefore blind to an entire configuration by construction, in exactly the way #209 exists to prevent. The CI crdt job — added by that lane — is what caught this, three days after it landed. Baselined rather than assumed. A worktree at the branch base 21de0b2 sweeps 3715 passed / 0 failed under `luajit,crdt` with PMACS_REQUIRE_GPU=1; the branch with this fix sweeps 3715 / 0. Identical. An earlier branch sweep, taken before this fix, reported SEVENTY failures across THIRTEEN suites. Twelve of those suites — m5_5, gpu_invocation, gpu_initial_target, m10_11, vterm_stage3 and the rest — are daemon, socket or GPU suites, and all of them recovered by fixing three compile tests. The likeliest reading is that the failing compile-crdt runs leaked daemon or PTY processes that poisoned every subsequent socket-based suite; what is ESTABLISHED is narrower and still useful: baseline green, branch green with the fix, and the collateral confined to process-spawning suites. Count failures, not causes — the same lesson this stage already learned once, at a different layer. Also of note: CI reported only 3 failures because `cargo test` halts after a failing binary. The workflow does not pass `--no-fail-fast`, so CI under-reports a multi-suite break exactly as my first census did. P2 — builtin/runtime/compile.lua's recompile comment still said `_last` stores only cmdline/cwd. This PR deliberately stores `display` too, so an explicit opt-out survives replay. Corrected, with the `display_omitted` arm's remaining purpose stated rather than implied. P2 — the framing's §7 step 2 still called the resolver extraction "provably behaviour-preserving", contradicting §1.6b's own record of the intentional non-string normalization. It is DEFAULT-PRESERVING WITH ONE INTENTIONAL NORMALIZATION, and now says so where a reader following the branch plan will hit it. Verified: fmt, diff-check, compile_mode_crdt 3/3, compile_mode 73/73, and the full crdt sweep at 3715/0 against a measured baseline of 3715/0. Co-Authored-By: Claude Opus 5 (1M context) --- builtin/runtime/compile.lua | 15 ++++++++++----- docs/bottom-panel-stage3-framing.md | 9 ++++++--- tests/compile_mode_crdt_acceptance.rs | 13 ++++++++++--- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index 20300cc..22fc643 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -893,11 +893,16 @@ local function start_run(slot, cmdline, opts) -- not preempt the requested panel. Compile output is passive, so it -- takes `select = false` explicitly. -- - -- A recompile reaches here with NO `display` (only cmdline/cwd are - -- stored in `_last`), so the raw switch below would put this buffer in - -- the selected DOCUMENT window while the panel still shows it — the - -- duplicate presentation this arc removes elsewhere. Detect that the - -- buffer already owns the panel slot and keep it there. + -- A recompile REPLAYS `_last`, which since Stage 3 carries `display` + -- alongside cmdline/cwd — an opt-out that did not survive replay + -- would silently revert to the panel on the next `g`. So an explicit + -- `display = "current"` reaches here again on a recompile, and must + -- still take the raw switch below. + -- + -- The `display_omitted` arm remains for the genuinely omitted case: + -- it keeps a buffer that already owns the panel slot in the panel + -- rather than duplicating it into the selected DOCUMENT window while + -- the panel still shows it. -- -- Gated on OMISSION, never on an explicit value: `display = "current"` -- is the documented user-facing opt-out from the Stage 3 default flip, diff --git a/docs/bottom-panel-stage3-framing.md b/docs/bottom-panel-stage3-framing.md index fa59cf2..18ba3b3 100644 --- a/docs/bottom-panel-stage3-framing.md +++ b/docs/bottom-panel-stage3-framing.md @@ -572,9 +572,12 @@ One branch, `bottom-panel-stage3`: measurement is the first commit's evidence, not the flip.* Classify each mover per §1.6a's rule before writing a line of the fix. 2. **Land `resolve_adopter_display`** (Q#S3-1) with all four callers - still passing their CURRENT defaults, so the unification is provably - behaviour-preserving before anything flips. Decide and pin the - non-string normalization here. + still passing their CURRENT defaults, so nothing flips yet. It is + **default-preserving with one intentional normalization**, not + "behaviour-preserving" — every adopter keeps its default and the + suite is byte-identical to baseline, but terminal's invalid-input + behaviour moves deliberately (§1.6b). Decide and pin that + normalization here, since no existing assertion can catch it. 3. **Flip the three sites** by changing only the `default` argument at listview, compile and terminal — dired keeps `"current"` — with per-adopter `select`. diff --git a/tests/compile_mode_crdt_acceptance.rs b/tests/compile_mode_crdt_acceptance.rs index ba79d49..a6970d9 100644 --- a/tests/compile_mode_crdt_acceptance.rs +++ b/tests/compile_mode_crdt_acceptance.rs @@ -105,6 +105,13 @@ where /// the daemon broadcasts a snapshot for the newly-CRDT-backed buffer /// and via the active-buffer-follow path). Re-seats the replica's /// mirror on that buffer. +/// +/// **Bottom-panel Stage 3:** this is why the runs below pass an explicit +/// `display = "current"`. The default now places compile output in the +/// panel with `select = false`, so the ACTIVE buffer never becomes +/// `*compilation*` — and the active-buffer-follow path named above is +/// what publishes the snapshot this function waits for. The subject +/// here is CRDT convergence of a generated buffer, not placement. fn adopt_next_buffer(replica: &mut Replica, what: &str) { let deadline = std::time::Instant::now() + Duration::from_secs(10); loop { @@ -188,7 +195,7 @@ fn compile_run_converges_and_replica_edit_triggers_recovery() { name = "test.compile", description = "compile-mode CRDT fixture trigger", fn = function() - pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }}) + pmacs.compile.run("sh {script}", {{ cwd = "{dir}", display = "current" }}) end, }} pmacs.keymap.bind {{ scope = "global", sequence = "C-c 9", command = "test.compile" }} @@ -275,7 +282,7 @@ fn r3f1_unicode_cr_backspace_survive_crdt_replication() { name = "test.compile-unicode", description = "round-3 unicode fixture trigger", fn = function() - pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }}) + pmacs.compile.run("sh {script}", {{ cwd = "{dir}", display = "current" }}) end, }} pmacs.keymap.bind {{ scope = "global", sequence = "C-c 8", command = "test.compile-unicode" }} @@ -327,7 +334,7 @@ fn r4f1_column_rewrites_replicate_and_converge() { name = "test.compile-columns", description = "round-4 column-rewrite fixture trigger", fn = function() - pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }}) + pmacs.compile.run("sh {script}", {{ cwd = "{dir}", display = "current" }}) end, }} pmacs.keymap.bind {{ scope = "global", sequence = "C-c 7", command = "test.compile-columns" }}