review round 1: the crdt suite Stage 3 missed, and two stale explanations
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) <noreply@anthropic.com>
This commit is contained in:
parent
480158b460
commit
6b4d52a5bc
|
|
@ -893,11 +893,16 @@ local function start_run(slot, cmdline, opts)
|
||||||
-- not preempt the requested panel. Compile output is passive, so it
|
-- not preempt the requested panel. Compile output is passive, so it
|
||||||
-- takes `select = false` explicitly.
|
-- takes `select = false` explicitly.
|
||||||
--
|
--
|
||||||
-- A recompile reaches here with NO `display` (only cmdline/cwd are
|
-- A recompile REPLAYS `_last`, which since Stage 3 carries `display`
|
||||||
-- stored in `_last`), so the raw switch below would put this buffer in
|
-- alongside cmdline/cwd — an opt-out that did not survive replay
|
||||||
-- the selected DOCUMENT window while the panel still shows it — the
|
-- would silently revert to the panel on the next `g`. So an explicit
|
||||||
-- duplicate presentation this arc removes elsewhere. Detect that the
|
-- `display = "current"` reaches here again on a recompile, and must
|
||||||
-- buffer already owns the panel slot and keep it there.
|
-- 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"`
|
-- Gated on OMISSION, never on an explicit value: `display = "current"`
|
||||||
-- is the documented user-facing opt-out from the Stage 3 default flip,
|
-- is the documented user-facing opt-out from the Stage 3 default flip,
|
||||||
|
|
|
||||||
|
|
@ -572,9 +572,12 @@ One branch, `bottom-panel-stage3`:
|
||||||
measurement is the first commit's evidence, not the flip.* Classify
|
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.
|
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
|
2. **Land `resolve_adopter_display`** (Q#S3-1) with all four callers
|
||||||
still passing their CURRENT defaults, so the unification is provably
|
still passing their CURRENT defaults, so nothing flips yet. It is
|
||||||
behaviour-preserving before anything flips. Decide and pin the
|
**default-preserving with one intentional normalization**, not
|
||||||
non-string normalization here.
|
"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
|
3. **Flip the three sites** by changing only the `default` argument at
|
||||||
listview, compile and terminal — dired keeps `"current"` — with
|
listview, compile and terminal — dired keeps `"current"` — with
|
||||||
per-adopter `select`.
|
per-adopter `select`.
|
||||||
|
|
|
||||||
|
|
@ -105,6 +105,13 @@ where
|
||||||
/// the daemon broadcasts a snapshot for the newly-CRDT-backed buffer
|
/// the daemon broadcasts a snapshot for the newly-CRDT-backed buffer
|
||||||
/// and via the active-buffer-follow path). Re-seats the replica's
|
/// and via the active-buffer-follow path). Re-seats the replica's
|
||||||
/// mirror on that buffer.
|
/// 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) {
|
fn adopt_next_buffer(replica: &mut Replica, what: &str) {
|
||||||
let deadline = std::time::Instant::now() + Duration::from_secs(10);
|
let deadline = std::time::Instant::now() + Duration::from_secs(10);
|
||||||
loop {
|
loop {
|
||||||
|
|
@ -188,7 +195,7 @@ fn compile_run_converges_and_replica_edit_triggers_recovery() {
|
||||||
name = "test.compile",
|
name = "test.compile",
|
||||||
description = "compile-mode CRDT fixture trigger",
|
description = "compile-mode CRDT fixture trigger",
|
||||||
fn = function()
|
fn = function()
|
||||||
pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }})
|
pmacs.compile.run("sh {script}", {{ cwd = "{dir}", display = "current" }})
|
||||||
end,
|
end,
|
||||||
}}
|
}}
|
||||||
pmacs.keymap.bind {{ scope = "global", sequence = "C-c 9", command = "test.compile" }}
|
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",
|
name = "test.compile-unicode",
|
||||||
description = "round-3 unicode fixture trigger",
|
description = "round-3 unicode fixture trigger",
|
||||||
fn = function()
|
fn = function()
|
||||||
pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }})
|
pmacs.compile.run("sh {script}", {{ cwd = "{dir}", display = "current" }})
|
||||||
end,
|
end,
|
||||||
}}
|
}}
|
||||||
pmacs.keymap.bind {{ scope = "global", sequence = "C-c 8", command = "test.compile-unicode" }}
|
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",
|
name = "test.compile-columns",
|
||||||
description = "round-4 column-rewrite fixture trigger",
|
description = "round-4 column-rewrite fixture trigger",
|
||||||
fn = function()
|
fn = function()
|
||||||
pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }})
|
pmacs.compile.run("sh {script}", {{ cwd = "{dir}", display = "current" }})
|
||||||
end,
|
end,
|
||||||
}}
|
}}
|
||||||
pmacs.keymap.bind {{ scope = "global", sequence = "C-c 7", command = "test.compile-columns" }}
|
pmacs.keymap.bind {{ scope = "global", sequence = "C-c 7", command = "test.compile-columns" }}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue