diff --git a/docs/active-work.md b/docs/active-work.md index 6c5aa66..a48aba3 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -301,66 +301,214 @@ waits for a signal that is not coming. - **THE FIRST DISPATCH IMMEDIATELY FOUND A RED ON `main`**, which is what this lane was built for. See the proptest entry below. -## CRDT identity-replace undo — the proptest invariant may be MIS-SCOPED — NEEDS A LANE +## CRDT identity-replace undo — LANE TAKEN, PR #246 OPEN -**CORRECTION.** An earlier version of this entry called the dispatched -run's red "a DETERMINISTIC red" and "not like anything else in this -registry — a property violation with a concrete witness, not a load -artefact", and proposed committing the proptest seed as the first -step. **All of that was wrong**, and it was wrong because I recorded a -finding without checking whether `main` already documented it. +**Branch `crdt-identity-undo`, PR #246, based on `aae5b35`.** Framing +`docs/crdt-identity-undo-framing.md`, **APPROVED at revision 4** after +four review rounds, then **revision 5** as a correction pass answering +implementation review. -**It is a randomly sampled recurrence of known #157 behaviour.** -`src/buffer.rs:3005` carries an `#[ignore]`d deterministic fixture, -`crdt_undo_of_an_identity_replace_reports_a_no_op_edit_carrying_an_op`, -which reduces this exact case and records its mechanism: +**Every head of this branch is gated head-exact**, with `HEAD` and +`git status --porcelain` captured before and after each run and +identical. The two that matter: -1. the inserts produce `aaaaa `; -2. `Replace(5, 1, " ")` replaces the trailing space **with itself** — a - textual no-op but a real CRDT delete-plus-insert; -3. `Undo` therefore emits a **version-advancing CRDT operation with no - visible text change**, and `derive_replacement_edit` yields an empty - edit that still carries its `crdt_op`. +| commit | what it carries | gate | CI | +|---|---|---|---| +| `2c24303` | the code, and the framing at revision 5 | all 8 green, `20260830T193305Z-4167110`, loadavg 0.90 | 14/14 | +| `6ddce0f` | the registry corrections (U14/U15 split, load claim narrowed) | all 8 green, `20260830T201908Z-84597` | 14/14 | -**Committing the seed is NOT the first step**, and proposing it was a -second error: it duplicates a deterministic fixture that already -exists, and its only effect would be to make a disputed assertion fail -permanently instead of occasionally. +**This line goes stale the moment another commit lands, which is the +defect review already caught here once** — it named `db24ae3` long after +the branch had moved past it. It is written as a table so the next +update is an added row rather than a rewrite. -**What the fixture already verified**, so the lane does not redo it: -content stays correct (rope and CRDT projection agree), replicas stay -converged (**the op IS broadcast** — both `crdt_op` consumers read it -unconditionally and neither short-circuits on an empty range), and the -cursor does not jump. +**The decision, ruled:** a visible TEXT delta and a CRDT-VERSION delta +are **independent dimensions** of `Edit`. The invariant is keyed on +**provenance**, and enumerated over three axes rather than defaulted: +an **empty text delta** is a shape both paths reach, and `crdt_op` is +what separates them — `None` forward (the three syntactically empty +`EditOp` forms short-circuit at `is_no_op_edit`), `Some` from +`undo`/`redo`, **required** there, because the op is the whole content +of such an edit. -**THE ACTUAL DECISION** is whether a **visible text delta** and a -**CRDT-version delta** are independent dimensions of `Edit`. The -proptest's invariant assumes they are the same dimension. It was -written for the FORWARD `apply_edit` short-circuit, which returns -before producing an op at all — and **CRDT-mode undo/redo never reach -that path**. +**Revision 5 fixed three things review caught in the implementation:** -**What the lane owes as evidence:** +- the predicate **conflated the empty text delta with a version delta**, + calling every empty-range/zero-insertion edit `version_only` and then + accepting `(History, empty, None)` through a wildcard — contradicting + the lane's own "the op must survive". It is now a full enumeration, + and C5 asserts all four empty-delta quadrants instead of two. **Both + new quadrants were mutation-checked, and neither is caught by the + proptest** — no generated input reaches either; +- the **public `Edit` doc was factually false**: it said forward + `apply_edit` never produces the empty-delta shape, while C2b proves + all three forward empty forms do; +- **R7's write-up overstated what the paired gate runs exclude.** They + exclude the source tree. They do not narrow the cause to three + candidates — scheduler load, kernel and socket timing, and unrelated + machine state all varied too, and a `BrokenPipe` on a socket handshake + is exactly what those can drive. -- **forward textual no-ops still produce NO operation** — whatever the - resolution, the short-circuit the invariant was actually written for - must keep holding; -- **any permitted empty-text undo operation carries valid bytes and - preserves remote replay convergence** — permitting the shape must not - become permitting a malformed op; -- **an explicit disposition of the arbitrary artifact**: - `derive_replacement_edit` reports the empty range at the **buffer - end** rather than at the edit site. The fixture calls this genuinely - arbitrary either way; the lane must say which it is choosing, not - leave it unexamined. +**A GATE COVERAGE GAP, found the expensive way.** The local gate's +clippy step is `cargo clippy --workspace --all-targets -- -D warnings` +— **default features**, so **no `#[cfg(feature = "crdt")]` code is ever +linted locally**. CI lints it (`--no-default-features --features +luajit,crdt`), so a crdt-only lint passes eight green gate stages and +then reds `Test (crdt)`. That is what happened here: a +`clippy::match_same_arms` on the new enumeration, invisible to five +consecutive local gate runs. -**Un-ignoring that fixture is the first step of whichever resolution -wins**, as its own doc comment says. +The lint itself is `#[allow]`ed with a reason — collapsing the three +`Ok(())` arms is exactly the conflation this lane removes, and would +hide that `(forward, empty, None)` and `(history, empty, Some)` are +valid for opposite reasons. **The gap is not fixed here**: adding a +second clippy flavor to `scripts/gate` is a change to shared +infrastructure and belongs in its own lane. Recorded so the next lane +touching crdt-gated code does not rediscover it at CI. -**Why it is worth a bounded interruption:** a mis-scoped property can -now randomly redden `main`, and the dispatched run proved it. **It does -NOT reorder the roadmap** — GUI arc 1b remains the next product lane -per `COHERENCE.md` §20's priority order. +**NINE registry rows moved on this lane** — R7, R6, U6, U14, U15, U16, +U17, U18 and U19. *(This heading said "four" while listing more; corrected.)* + +- **R7's eleventh and TWELFTH occurrences** — the eleventh is the + green/red pair whose heads differ by one markdown file, which excludes + the SOURCE TREE and nothing more; an earlier write-up of mine narrowed + the cause to three gate-state candidates and that overstatement is + withdrawn in the row. **The row's numbering was wrong twice over**: it + carried TWO blocks labelled "fourth" (D3 2026-08-11, TMPDIR + 2026-08-13), **and** two full-fragment occurrences of 2026-08-15 sat + in this file marked "owed to the registry" and were never absorbed. + Renumbered by date with both fixed, and the row's summary now states + the total and which `attach.rs` line each group reports; +- **U6 went from one occurrence to five** — four on 2026-08-30, two out + of gate and two in. Its first reproduction ever. **A direction claim I + made here ("runs the OPPOSITE way to R7", resting on four green + `04-lib-crdt` stages) was falsified by the next gate run and is + withdrawn in the row;** +- **U14, new** — four selectors across **four** unrelated subsystems + (async runtime, optimistic orchestration, editor composition, LSP + dispatch) red in one gate run, spread over three stages; +- **U15, new** — the rotated cluster 40 minutes later. It carries a + single `/proc/loadavg` reading of **34.04**, which makes severe + unrelated load a **measured presence contemporaneous with a multi-red + run — not a measured cause.** The reading is one point taken after the + fact and the margins are not monotonic + (`composition_overhead` ran 1.182x, 1.592x, 1.527x), so no + dose-response is claimed. **It is the first contemporaneous load + reading for a U6 occurrence, and a second data point beside the one + U7 has carried since 2026-08-09** — not this registry's first. Two + earlier versions of that write-up overreached: one said a load average + of 34 "explains it without any help", the other that U6 and U7 had + both wanted a number since August. Both are corrected in place. + +- **U16, new** — a `git` invocation in `packages::fetcher` found its + working directory **deleted**. Not a budget: the only row in the + registry that arrives with a **named candidate mechanism inside the + test suite**, and the load-bearing step is **child inheritance**. + `src/file_io.rs:434` mutates process-global cwd; concurrently + `run_git` calls `run_git_inner(None, …)`, which sets `current_dir` + only when `cwd` is `Some` (`fetcher.rs:329`–`:330`), so the spawned + `git` **inherits** the temp cwd. **Restoring the parent's cwd does + nothing for that child**, and the `TempDir` then drops underneath it. + Candidate, not a demonstrated chain — 8 full parallel `--lib` runs did + not reproduce it, which establishes **intermittence and nothing + more**. The controls that would settle it are in the row and **none is + run here**; note that a serial guard around `set_current_dir` tests is + *not* among them, since the child outlives the guard. The structural + fix belongs to whoever owns `file_io`, not to a CRDT invariant lane. + +- **R6's SECOND occurrence** — 26 days after the first, macOS `lua54`, + a **full three-condition match** including both required fragments. + **The log was read before anything was rerun**, which is U3's lesson + and U8's fourth-violation warning finally honoured on a macOS job. A + **merge-base control was dispatched** at `aae5b35` rather than + arguing from an unrelated diff. **Not the `workflow_dispatch` key's + first use** — #245's own D2/D3 witnesses dispatched three runs right + after it merged — but **the first use for a live merge-base + control**, which is the case U11 motivated it for. It came back **green on the macOS legs**, so the + inference it could have supplied is **unavailable**; recorded as a + null result, as R1's row had to record its own; +- **U16 REPRODUCED** — second occurrence 2026-08-31, same step, same + fragments. It settles the earlier withdrawal in the right direction: + "the window is narrow" was wrong to claim from eight green runs, and + the failure returned within the day. The child-inheritance chain + stays a candidate; this occurrence demonstrates it no more than the + first did. **It then passed in all three stages of the next gate + run** — two reds and many greens **all on 2026-08-31**, intermittent at + a rate nothing has measured. +- **U19, new** — `terminal_bell_baseline_…` timed out on a **5-second** + poll, in the same run as U16's second. The evidence is that **no bell + was observed within five seconds** — not that one never came, and not + that scheduling cannot explain it. A much slacker deadline than the + budget family's (5000× the 1ms budget, but only **25×** the 200ms + one), so the distinction is of degree rather than kind. Like R1, **its + assertion reports no elapsed value**, so **this** margin is gone for + good; adding it later would make future occurrences comparable **to + each other**, not to this one. **It passed in all three stages of the + next gate run**, so it is intermittent. +- **U18, new** — `Test (ubuntu-latest / luajit)` died in **toolchain + setup**, before any `cargo` command ran: `go install gopls@v0.16.2` + hit `INTERNAL_ERROR` from `sum.golang.org` while verifying + `x/telemetry`. A new class for the registry — every other row is a + test that failed; this is infrastructure the workflow depends on + failing to answer, and it presents as a red check indistinguishable + from a real one. No attribution to the branch is possible: the step + precedes compilation, and the other 13 checks passed on the same head. + **Not rerun** — a transient network error is expected to pass on + retry, which would establish nothing. +- **U17, new** — that same control run **redded `Test (crdt)` on `main` + at `aae5b35`**: `read_dir_supersede_cancels_in_flight_predecessor`, + `first read_dir must be superseded; got ok`. It fails the **opposite** + way to R1 and R5 — not a missed deadline. What `got ok` proves is + narrow: the predecessor **completed successfully before cancellation + took effect**, which does not say when the supersede arrived. The job + runs `--test-threads=1`, which serializes test **functions within one + executable** — **not** the test-**binary** concurrency U9's control + named, and cargo runs binaries serially anyway. A PR run could show + this failure too; what only a `main`-side run establishes is that it + fails **on `main`**, with no observing branch to suspect. + +U14 and U15 are two rows rather than one because the second run's +selector set had **rotated**, and this registry matches on the exact +set — recording it as a second U14 occurrence was a matching-rule +violation, caught in review. + +**Two claims THIS BLOCK made are corrected by measurement:** + +- it said the fixture had verified that *"replicas stay converged — the + op IS broadcast"*. **That was inspection of the call sites, not + execution.** Nothing had ever replayed the op on a replica, and text + equality alone cannot detect a lost version advance — the drop-the-op + mutant leaves the text identical, and the failure that catches it + reads `version vector diverged undo`. C3 establishes convergence + properly, by seeding a replica with the forward ops first; +- it said the buffer-end range location was *"genuinely arbitrary either + way"*. **The §4 census rules it**: five consumers inert, three + permitted, none harmed — and for `TextView`, the one whose cost + depends on the location, the buffer end is the **cheapest** rebuild. + +**What the four review rounds caught, none of it by me.** Revision 1 +posed the decision instead of answering it, and its C3 passed its own +drop-op mutant. Revision 2's C4 contradicted the implementation +(`mark_stale` is unconditional and range-independent) and named two +consumers out of six. Revision 3's C4 claimed guard mutations that +**survive** — at the buffer end, deleting the fold or style guard +changes nothing — and its C9 guarded a file set and a count, which a +same-file substitution walks straight through. + +**15 mutation checks were run, each on a clean tree and reverted.** All +behaved as the framing predicted, including the two asymmetries the +framing states rather than assumes: C2b is **masked** for the Insert and +Delete forms by their defensive early returns (`buffer.rs:1177`, +`:1192`) and **dies** for Replace, which has none; and C4b **survives** +the style-guard deletion, which is why C4c injects an INTERIOR empty +edit where the fragmenting is reachable. + +**Deliberately not done:** the proptest regression seed is NOT +committed. It duplicates a deterministic fixture and would make a +disputed assertion fail permanently rather than occasionally. + +**It does NOT reorder the roadmap.** GUI arc 1b remains the next product +lane per `COHERENCE.md` §20. ### Superseded lane state, kept for the record @@ -428,7 +576,7 @@ from #171 and #215. U13 is not this lane's and is left as recorded, but the pattern is now a pattern rather than an oversight, and each occurrence costs a review round to establish nothing. -- **R7 gained its sixth and seventh occurrences here**, on a branch +- **R7 gained its ninth and tenth occurrences here** (recorded at the time as its sixth and seventh; the row was renumbered on 2026-08-31 after two duplicate ordinals and two unabsorbed 2026-08-15 occurrences were found), on a branch touching no `pmacs-gpu` file, and the registry gained a **bounded observation window** so the in-gate/out-of-gate ratio cannot drift with review activity — plus a correction: seventeen green @@ -518,10 +666,14 @@ from #171 and #215. the full 36-test binary both passed immediately afterwards — intermittence only. This lane changes neither the gate script nor that acceptance binary; diagnostic hardening is a separate lane. -- **Still owed, separately:** `workflow_dispatch` on `ci.yml`, and U9's - discriminating control — pin test-binary concurrency to 1, then load - a lone `--lib` binary — which has been named since 2026-08-09 and - never run. +- **Still owed, separately:** `workflow_dispatch` on `ci.yml` (**landed + as #245**), and U9's discriminating control — named since 2026-08-09, + never run, and now **VOID**: its premise that `cargo test --workspace` + runs many test binaries at once is false, cargo runs test targets + serially, so "pin test-binary concurrency to 1" pins something already + 1. See the correction on U9 in `docs/ci-red-signatures.md`. **A + replacement control has to be designed**; the budget family no longer + has one written down. ## Panel-pointer replay (parent acceptance 48) — MERGED as #243 (`6c9bae6`) @@ -1227,8 +1379,12 @@ from #171 and #215. branches' entries "merged **without a conflict**, producing duplicate ids across four sites". **The rows below are owed to the registry by whichever branch merges second**, numbered after the other's. - - **R7, two occurrences on this branch** (2026-08-15, `gpu` step, - logs `20260815T095532Z` and `20260815T100719Z`). Fragments verified + - **R7, two occurrences on this branch — ABSORBED 2026-08-31 as the + row's seventh and eighth** (2026-08-15, `gpu` step, + logs `20260815T095532Z` and `20260815T100719Z`). **They sat here + unabsorbed for sixteen days** while both branches merged, which is + why R7's count read two low; the deferral was reasonable, not + discharging it was not. Fragments verified both times: `transient sequence must attach: Attach(Handshake(Io(Os { code: 32, kind: BrokenPipe, message: "Broken pipe" })))` at `pmacs-gpu/src/attach.rs:1728`. One machine, one day, one branch, @@ -1407,9 +1563,15 @@ from #171 and #215. load average **14.02 → 28.35**, from an unrelated `turso` test suite on the same machine (`./verify_task_state.sh turso-without-rowid`, target dir `/opt/target`, one test binary at - **693% CPU**). It is not a controlled experiment, but it is the - same evidence U9's synthetic-load control was meant to produce, and - it points at load. U9 stays owed; its value is now lower. + **693% CPU**). It is not a controlled experiment, and **an earlier + version of this bullet called it "the same evidence U9's + synthetic-load control was meant to produce" and said "U9 stays + owed". Both are withdrawn.** Uncontrolled foreign load is *not* the + same evidence as an experiment that applies load deliberately, and + U9's control is now **VOID** in its concurrency arm and a + non-discriminator in its load arm — see the correction on U9 in + `docs/ci-red-signatures.md`. What this reading is: a **named + confound**, recorded, pointing at load without establishing it. - **Two process traps this cost, both worth carrying forward.** The Bash tool caps a command at 10 minutes and SIGTERMs it, which the gate reports as `FAILED (exit 143)` on whatever stage was running — @@ -1427,9 +1589,14 @@ from #171 and #215. only: no mechanism is claimed, and the standing leaked-daemon confound is uncontrolled as always. - **Cost, stated plainly:** four `--protocol` gate runs on one commit, - three of them lost to these two signatures. U9's synthetic-load - control remains unrun and is the cheapest thing that would either - implicate load or clear it. + three of them lost to these two signatures. **This sentence used to + add that U9's synthetic-load control "would either implicate load or + clear it". It would not.** With cargo running test targets serially, + U9's concurrency arm is void and there is no second arm to compare + against: a red under synthetic load shows load is **sufficient**, and + a green shows nothing — non-reproduction never clears anything under + this file's own rerun rule. See the correction on U9 in + `docs/ci-red-signatures.md`. - **Rustdoc split, FOUR occurrences on this branch** (`screen_size`, `peer_may_send_panel_events`, `send_panel_pointer`, and `SemanticRenderState`). Always the same mechanism: inserting an item diff --git a/docs/ci-red-signatures.md b/docs/ci-red-signatures.md index 99a5f79..8546a42 100644 --- a/docs/ci-red-signatures.md +++ b/docs/ci-red-signatures.md @@ -239,10 +239,70 @@ happened and had a signature, which is the entire bar for a row. | **selector** | `--test bottom_panel_stage1_acceptance acc28_child_input_and_the_c_c_escape_work_unchanged_in_a_panel` | | **job / flavor** | macOS / lua54 | | **required fragments** | `timed out waiting for` **and** `/ready` | -| **causal status** | **UNRESOLVED — no diagnosis** | -| **evidence** | #217 [run 31023651701](https://github.com/levineuwirth/pmacs/actions/runs/31023651701), 2026-08-05 | +| **causal status** | **UNRESOLVED — no diagnosis. SECOND OCCURRENCE 2026-08-31**, 26 days after the first | +| **evidence** | #217 [run 31023651701](https://github.com/levineuwirth/pmacs/actions/runs/31023651701), 2026-08-05. Second: PR #246 [job 99431791766](https://github.com/levineuwirth/pmacs/actions/runs/33374169011/job/99431791766), head `e87d227` | | **retirement** | the readiness helpers are audited and reconciled, with a witness. **Never a green rerun** — the next push was green and that retires nothing. | +**Second occurrence — the CRDT identity-undo lane, 2026-08-31, +`Test (macos-latest / lua54)` on PR #246.** **A full three-condition +match**, which this file requires and which is worth spelling out +because the last occurrence of this selector could not be matched at +all: + +1. **selector** — `acc28_child_input_and_the_c_c_escape_work_unchanged_in_a_panel`; +2. **job / flavor** — macOS / `lua54`, the same leg; +3. **both required fragments** — `timed out waiting for /var/folders/df/djsxfhc17x95674wsm_g8s980000gn/T/.tmpyBcHeZ/ready`. + +Panic at `tests/bottom_panel_stage1_acceptance.rs:2454`, +`46 passed; 1 failed`. + +**The log was read BEFORE anything was rerun.** U3 named that lesson, +U8 recorded its fourth violation, and this is the first occurrence in +this file's history where the rule was followed on a macOS job at the +moment it mattered. The fragments above exist because of it. + +**Not attributed to the observing lane, and a merge-base control was +dispatched rather than argued.** The branch's whole diff is +`src/buffer.rs`, `src/rope.rs`, `src/overlay.rs`, `src/view.rs` (tests +and doc comments) plus three docs — **no file under `tests/`, and +nothing in the panel, process or terminal paths**. But "my diff looks +unrelated" is not evidence, so +[run 33375945966](https://github.com/levineuwirth/pmacs/actions/runs/33375945966) +was dispatched at `aae5b35`, **the branch's exact merge base**, via the +`workflow_dispatch` key #245 landed for precisely this. + +**It is NOT that key's first use, and an earlier version of this block +said so.** #245's own owed witnesses D2 and D3 dispatched three runs +(`33307137965`, `33308891808`, `33308921103`) immediately after it +merged, and the first of those already found a red on `main`. What this +is: **the first use for a live merge-base CONTROL** — a contemporaneous +`main`-side run obtained to answer a specific branch-side red, which is +the case U11 motivated the key for. + +**THE CONTROL LANDED GREEN on the macOS legs**, and the meaning was +pre-registered above before the result was seen: +`Test (macos-latest / lua54)` **succeeded** at `aae5b35`. So the +inference this control could have supplied — that the branch did not +introduce the failure — **is unavailable**. What is established is only +that the merge base can pass the same job in the same hour. That is +exactly what R1's row had to record about its own green control, and it +is recorded the same way here: **a null result, not an exculpation.** + +**The control run was not otherwise clean, and that is its own finding.** +`Test (crdt)` **failed on `main` at `aae5b35`** — see **U17**. **A PR +run can show the identical red**, and an earlier version of this block +denied it; what only the `main` dispatch establishes is that the failure +occurred **on the merge base**, with no observing branch to suspect. + +**Circumstantial alignment with U8, deliberately NOT a merge.** U8 has +the same selector, panicking at the **same line** `:2454` with the +**same** `46 passed; 1 failed`, on macOS `luajit` at base `0190102`. +That is suggestive. It is also unconfirmable: **U8's fragments were +destroyed**, and a row with no fragments cannot be matched — which is +exactly what U8's own entry says it is for. The alignment is recorded +here so a future reader sees it; U8 stays a separate row, and the +inference stays unavailable. + **A THIRD copy of the readiness helper.** R4's disposition already recorded that the empty-file predicate lived in a second helper (`wait_for_published_file`) and warned that leaving it would let the @@ -535,13 +595,96 @@ Stage 4; the lane touches no `pmacs-gpu` code at all. | **selector** | `-p pmacs-gpu attach::tests::managed_retry_survives_transients_and_uses_the_successful_stream` | | **job / flavor** | local (Linux), `cargo test --workspace --features crdt --no-fail-fast`, i.e. under full-sweep load | | **required fragments** | `transient sequence must attach` + `Handshake(Io(` + `BrokenPipe` (or `code: 32`) | -| **status** | **SEVENTH OCCURRENCE 2026-08-29 — causal status still UNRESOLVED.** The sixth and seventh came back to back on one lane and are written up together below; the fifth carries the strongest tree exclusion this row has had, a **documentation-only diff** | -| **what IS established** | **three** occurrences at `pmacs-gpu/src/attach.rs:1680`, the second and third with all three fragments **verified** rather than inferred; the test drives a scripted transient-then-success sequence over a real socket pair. **The added GPU test is not the mechanism** — see the third-occurrence control below | +| **status** | **TWELFTH OCCURRENCE 2026-08-31 — causal status still UNRESOLVED.** The eleventh carries the strongest tree exclusion this row has had, and it supersedes the sixth's: two consecutive gate runs on ONE worktree whose heads differ by a single markdown file, the first all-green and the second red. **NOTE: the numbering was wrong twice over.** The row carried TWO blocks labelled "fourth" (D3 on 2026-08-11, TMPDIR isolation on 2026-08-13), and **two further full-fragment occurrences of 2026-08-15 sat in `docs/active-work.md` marked "owed to the registry" and were never absorbed** (logs `20260815T095532Z`, `20260815T100719Z`). Renumbered by date with both defects fixed; the count below is the total | +| **what IS established** | **TWELVE occurrences.** The line moves as `attach.rs` changes and this row treats a `:LINE` suffix as occurrence-specific: occurrences **one through four** report `pmacs-gpu/src/attach.rs:1680`; the **fifth records no line at all** — its block never captured one, and it is marked unrecorded rather than guessed; **six through eight** report `:1728`; **nine through twelve** report `:1889`. The second and third carry all three fragments **verified** rather than inferred; so do the seventh and eighth. The test drives a scripted transient-then-success sequence over a real socket pair. **The added GPU test is not the mechanism** — see the third-occurrence control below | | **what is NOT** | whether the broken pipe is the *fixture's* writer closing early or a real retry-path defect. **This row is not a claim that it is harmless** | | **rerun evidence** | occurrence 1: 6 isolated runs green, plus a full `--workspace --features crdt` sweep green (113 targets). Occurrence 2: **30 green on the observing branch** (15 isolated selector, 15 full `-p pmacs-gpu`) **plus a 15-run merge-base control, also green**. Occurrence 3: 5 isolated selector runs green, 10 full `-p pmacs-gpu` runs green **with** the added test, and **1 failure in 10 with the added test `#[ignore]`d** — the first rerun in this row's history that reproduced anything. Per the rerun rule the green runs establish intermittence only; the red control run is what carries the exclusion | | **retirement** | hardening that removes the named mechanism plus a discriminating witness — or a diagnosis showing the fixture, not the code, closes the pipe | -**Sixth occurrence — the parse-budget diagnosability lane, 2026-08-29, +**Twelfth occurrence — the same lane, 2026-08-31, local (Linux), `gpu` +step**, log `20260831T141818Z-2974002`. All three required fragments, +same selector, same `pmacs-gpu/src/attach.rs:1889`, same +`283 passed; 1 failed`. The other seven stages were green. + +**Recorded, and it adds nothing but a count — deliberately.** The +eleventh occurrence's method note says the remaining candidates have to be +varied **inside** the gate, one per run; that has not been done here and +this lane is not the place to start. `/proc/loadavg` at the failure read +`3.35 8.88 5.93` — a condition, recorded because U15 made load a +measured quantity in this file, **not** a cause, and R7 is not a budget +row. The observing commit is documentation only. + +**Eleventh occurrence — the CRDT identity-undo lane, 2026-08-30, local +(Linux), `gpu` step.** All three required fragments present in the +durable log +(`pmacs-fdccc423/gate-logs/20260830T155621Z-3005460/06-gpu.log`): + +``` +transient sequence must attach: Attach(Handshake(Io(Os { code: 32, +kind: BrokenPipe, message: "Broken pipe" }))) +``` + +at `pmacs-gpu/src/attach.rs:1889`, `283 passed; 1 failed`. + +**This occurrence discriminates tree from runner more sharply than any +before it, and the reason is the pair, not the diff.** Two consecutive +`scripts/gate` runs, same worktree, minutes apart: + +| run | head | delta from the previous run | result | +|---|---|---|---| +| `20260830T154827Z-2907414` | `db24ae3` | — | **all 8 stages green** | +| `20260830T155621Z-3005460` | `96bf2c3` | **one commit, touching one file: `docs/active-work.md`** | **`gpu` and `sweep` FAILED** | + +The sixth occurrence excluded the observing tree *relative to `main`* +by having a documentation-only diff. This pair excludes it relative to +**the immediately preceding green run of the same gate on the same +worktree**, where the entire delta is a markdown file that no Rust +target reads. Whatever varies between those two runs, it is not the +source tree. + +**`sweep` failed with the SAME single test**, so this run is one failure +surfacing in two stages, not two failures. Both stage logs name +`attach::tests::managed_retry_survives_transients_and_uses_the_successful_stream` +and nothing else: `283 passed; 1 failed` in each. + +* **Rerun: isolated selector green five times** (`1 passed`, 0.00s + each). Per this file's rerun rule that establishes **intermittence + only** — and per the tenth occurrence's correction, running the + selector outside the gate excludes nothing at all, because nothing + outside the gate has ever reproduced this failure. +* **No ratio is claimed from this occurrence.** It is one in-gate + failure following one in-gate pass, on a lane whose gate runs exist to + verify a head rather than to observe this row. Folding them into the + 2026-08-29 window would be exactly the drift that window was bounded + to prevent. +* **The observing lane touches no `pmacs-gpu` file**: its whole diff is + `src/buffer.rs`, `src/rope.rs`, `src/overlay.rs`, `src/view.rs` — tests + and doc comments — plus three docs. +* **The NEXT in-gate run was green**, all eight stages + (`20260830T160242Z-3095339`), one commit later. Recorded because + omitting it would be selective, not because it resolves anything: per + the rerun rule a green run establishes intermittence only, and the + tenth occurrence already falsified "in-gate always fails". + +**What this changes about the method, stated at the strength it +carries.** The tenth occurrence's narrowing said the remaining +candidates must be varied INSIDE the gate, one per run. This pair +sharpens **one** exclusion and nothing else: **the Rust source tree is +not the variable.** + +It does **not** narrow the cause to three things. The gate's per-run +state — its ambient root, its fresh `TMPDIR`, and process state carried +across stage boundaries — remains the set of candidates this project can +actually vary one at a time, which is why they are the ones to try. But +**they are not an exhaustive causal set**, and an earlier version of +this paragraph said they were. Everything unrelated to the repository +also differed between the two runs: scheduler load and CPU contention, +kernel and socket timing, page cache and memory pressure, and whatever +else the machine was doing at 15:48 versus 15:56. A socket handshake +racing a `BrokenPipe` is exactly the kind of failure those can drive, +and holding the tree fixed says nothing about any of them. + +**Ninth occurrence — the parse-budget diagnosability lane, 2026-08-29, local (Linux), `gpu` step.** All three required fragments present in the durable log (`pmacs-parse-budget-9c27ecfe/gate-logs/20260829T144541Z-350549/06-gpu.log`): @@ -553,7 +696,7 @@ kind: BrokenPipe, message: "Broken pipe" }))) at `pmacs-gpu/src/attach.rs:1889`, `283 passed; 1 failed`. -* **The tree exclusion is as strong as the fifth's.** The observing +* **The tree exclusion is as strong as the sixth's.** (Occurrences seven and eight, below, are on that same lane's branch.) The observing lane's entire diff is `src/async_runtime.rs`, `tests/m4_acceptance.rs` and three docs — **no `pmacs-gpu` file, and no file `pmacs-gpu` links against beyond the workspace it always did.** @@ -565,17 +708,22 @@ at `pmacs-gpu/src/attach.rs:1889`, `283 passed; 1 failed`. `07-sweep.log` ends in `Terminated`. That stage's absence says nothing, and the run as a whole is **not** a gate result. Only the `gpu` stage's failure is, because it completed and reported. -**A SEVENTH OCCURRENCE FOLLOWED IMMEDIATELY**, on the next gate run of +**A TENTH OCCURRENCE FOLLOWED IMMEDIATELY**, on the next gate run of the same worktree at head `45d438c` (`20260829T150011Z-429115/06-gpu.log`), same selector, same three fragments, `283 passed; 1 failed`. **That run's other seven stages were green**, `sweep` included and complete — 121 result lines, none with a failure — so this pair is not confounded by a truncation the way the -sixth was. +ninth was. **Two consecutive in-gate failures is new for this row**, whose prior -five were spread across lanes and months. It prompted a narrowing, and -the narrowing is the useful part. +**eight** were spread across lanes and weeks — **though not evenly**: +three of the eight fall on 2026-08-15, on one branch and one machine. +*(This sentence said "prior five … across lanes and months" until the +renumbering of 2026-08-31; it was written before the 2026-08-15 pair was +absorbed and before the duplicate "fourth" was found, and it was wrong +about the count and the spread.)* It prompted a narrowing, and the +narrowing is the useful part. **A THIRD IN-GATE RUN WAS GREEN** (head `68a16f9`, log `20260829T152024Z-563254`, all eight stages, zero failures anywhere). @@ -626,7 +774,31 @@ lanes and trees, and these locate the asymmetry in the *runner* while showing that the obvious way to probe it — reproducing gate conditions outside the gate — cannot work. -**Fifth occurrence — panel cell-mapping generation (§5b) framing, +**Seventh and eighth occurrences — the same §5b branch, later the same +day, 2026-08-15, local (Linux), `gpu` step.** Logs +`20260815T095532Z` and `20260815T100719Z`. **All three fragments +verified both times** — +`transient sequence must attach: Attach(Handshake(Io(Os { code: 32, +kind: BrokenPipe, message: "Broken pipe" })))` at +`pmacs-gpu/src/attach.rs:1728`, the same line as the sixth. One machine, +one day, one branch, **with a green full-gate run between them**. +Isolated reruns green. + +**These sat unabsorbed for sixteen days, and that is the finding worth +keeping.** `docs/active-work.md` recorded them under a heading saying +they were "**owed to the registry by whichever branch merges second**", +deliberately held back to avoid inventing a row id against an unseen +neighbour — a real hazard this file has been bitten by, when two +branches' entries merged without a conflict and produced duplicate ids +across four sites. **The deferral was reasonable; not discharging it was +not.** Both branches merged, and nothing carried them across, so R7's +count read two low until 2026-08-31. + +*The lesson is narrower than "absorb faster": an entry parked under +"owed to the registry" needs an owner named in the same sentence, or it +belongs to nobody.* + +**Sixth occurrence — panel cell-mapping generation (§5b) framing, 2026-08-15, local (Linux).** The `scripts/gate` **`gpu` step** again, the same flavor as occurrence 2, inside a `--protocol` run (log `20260815T072601Z-2230169`). @@ -647,9 +819,11 @@ the same flavor as occurrence 2, inside a `--protocol` run not exonerate the tree — though here there is no tree change to exonerate. -**What five occurrences now support, stated carefully:** the failure is -**not lane-correlated**. It has appeared under three flavors across -five unrelated lanes, once on a diff that touches no code whatsoever. +**What the first six occurrences supported, stated carefully** — and it +is unchanged by the six that followed: the failure is **not +lane-correlated**. It has appeared under three flavors across six +unrelated lanes by this point, once on a diff that touches no code +whatsoever. That is evidence about *where the cause is not*, and still says nothing about what it is. **The retirement condition is unchanged.** @@ -688,7 +862,7 @@ one-second deadline. Contention is a plausible mechanism for a lands, **run the control with the added test removed** rather than at the merge base — that is the discriminating comparison this one was not. -**Fourth occurrence — the `scripts/gate` TMPDIR isolation lane, +**Fifth occurrence — the `scripts/gate` TMPDIR isolation lane, 2026-08-13, local (Linux). Same selector, same `gpu`-step flavor (`PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu`), all three fragments verified** against the durable gate log @@ -725,7 +899,7 @@ with no new mechanism.** What it adds is the corroboration above. Three isolated re-runs on the current tree were green, which by this file's own rule establishes intermittence only. -**The discriminating comparison for a fifth occurrence** remains the +**The discriminating comparison for a SIXTH occurrence** remains the one the third occurrence prescribed. One occurrence, with no supported mechanism, is not grounds to reverse a fix that closes two observed hazards. @@ -763,7 +937,7 @@ the lane's only `pmacs-gpu` addition is the arm that went red. The next agent to touch this row should reproduce at 1-in-10 and instrument which side closes the pipe, rather than re-running for green. -**Fourth occurrence — D3 file-watch scheduler (PR #235), 2026-08-11, +**Fourth occurrence by date — D3 file-watch scheduler (PR #235), 2026-08-11, local (Linux), at the gate's SWEEP step** (`cargo test --workspace --no-fail-fast`, default features — U3's flavor, this time with the fragments captured). All three required fragments verified against the @@ -855,11 +1029,54 @@ was lost. | **selector** | `--lib --features crdt optimistic::tests::criterion_1_end_of_line_typing_completes_sub_frame_per_keystroke` **and** `editor::tests::composition_overhead_under_ten_percent`, failing in the same run | | **job / flavor** | local (Linux), `scripts/gate` step `04-lib-crdt`, with sibling worktrees building concurrently | | **required fragments** | `criterion 1: per-keystroke orchestrator time` + `exceeds 1ms`; and `composition machinery added more than 10% overhead` | -| **status** | **new incident, one occurrence, not reproduced** | +| **status** | **FIVE OCCURRENCES — one on 2026-08-09 and FOUR on 2026-08-30**, the first time this row has ever reproduced. Two of the four out of gate, two in gate (`20260830T171941Z`, `T175657Z`). Still no mechanism; see the block below | | **what IS established** | both are **wall-clock budget assertions** — 1.264ms against a 1ms budget, and 1.297× against a 1.10× budget — so both are load-sensitive by construction. Both green in an isolated rerun of exactly those two selectors, and both green in the next full gate run of the same command (2105 passed) | | **what is NOT** | whether the machine's concurrent load caused it. The confound is real (this machine runs one shared `CARGO_TARGET_DIR` and several worktrees) but **was not measured**, so it is a rival explanation, not a finding | | **rival explanation not excluded** | a genuine regression in either path. Nothing in the observing diff touches the optimistic-echo orchestrator or the composition pipeline, but "my diff looks unrelated" is not evidence, and this row does not treat it as such | +**Second occurrence — the CRDT identity-undo lane, 2026-08-30, local +(Linux).** Both required fragments captured, both selectors, one run: + +``` +criterion 1: per-keystroke orchestrator time 1.343883ms exceeds 1ms +composition machinery added more than 10% overhead: 1.182 +``` + +**It reproduced on the immediately following run**, which is new for +this row — the first occurrence explicitly recorded "not reproduced". + +Margins are recorded because U11 taught this registry what their absence +costs. Here: **1.343883ms against a 1ms budget** (1.34×) and **1.182× +against a 1.10× budget**; at the first occurrence, 1.264ms and 1.297×. +So the composition margin grew and the keystroke margin grew, but +neither by an order that separates load from regression. Both selectors +were green in isolated single-selector reruns. + +**Fourth and fifth occurrences — both IN gate, both `04-lib-crdt`, +both the exact pair.** `20260830T171941Z-3509751` (`1.689259ms` against +1ms; `1.592×` against 1.10×) and `20260830T175657Z-3881334` +(`2.269247ms`; `1.527×`). Those two runs also redded other selectors in +other steps; **those clusters are U14 and U15 respectively**, and only +the `04-lib-crdt` pair belongs to this row. + +**A DIRECTION CLAIM WAS MADE HERE AND IS WITHDRAWN, within the hour.** +This block first said the asymmetry "runs the OPPOSITE way to R7": +both failures were out of gate, while `04-lib-crdt` was green in all +four of this lane's gate runs to that point (`20260830T154827Z`, +`T155621Z`, `T160242Z`, `T160824Z`). **The very next gate run redded +`04-lib-crdt` with this exact pair.** So U6 fails **both** in and out of +gate, the four green stages were a run of four and not a property, and +the only honest reading is the one the first occurrence already gave: +these are wall-clock budget assertions. + +**No mechanism is claimed.** The load confound was unmeasured at the +second and third occurrences and at the fourth; **U15 records a single +`/proc/loadavg` reading of `34.04` taken after the fifth**, which makes +severe unrelated load a measured presence rather than a measured cause — +see that row for why the difference matters. What is worth having here +is that **this row is now reproducible under some condition**, which the +first occurrence could not say. + **Two budget tests failing in one run and neither in the next is the signature worth matching**, more than either name alone: a real regression in two unrelated subsystems at once is far less likely than @@ -1013,7 +1230,7 @@ claim is the one a later reader would otherwise reach for.* | **status** | **one occurrence; INTERMITTENT — the identical sweep command on the same tree was green (118 targets, 1928 passed, exit 0)** | | **what IS established** | intermittence, with the strongest available exclusion of the tree: green in two earlier steps of the **same run**, green isolated afterwards (`2 passed`, 1.70 s), green on a full sweep rerun. Both assertions are **timing-sensitive by construction** — one reads collected child output within a deadline, the other measures wall-clock composition overhead (observed 1.613× against a 1.10× budget; 61.3% dispatch and 124.6% realistic overhead) | | **what is NOT** | cause, and the load confound is **partially measured but NOT controlled**. The failing sweep ran inside a full gate; the green rerun started at load average 1.98 with the 5-minute figure still at 8.03 from that gate. Different conditions is not a measurement of the mechanism, and this row does not treat it as one | -| **the structural difference worth testing next** | `cargo test --workspace` runs **many test binaries concurrently**; `--lib` runs **one**. That is a difference in kind between the passing steps and the failing one, not merely a difference in load average — and it is the first candidate this family has had that is checkable rather than atmospheric. **Discriminating control:** rerun the sweep with test-binary concurrency pinned to 1, and separately run the `--lib` binary alone under synthetic load. A red under synthetic load at low sweep concurrency implicates load; a red at high concurrency and low load implicates the concurrency itself | +| **the structural difference worth testing next — PREMISE FALSIFIED 2026-08-31** | This cell claimed `cargo test --workspace` runs **many test binaries concurrently** while `--lib` runs one, and derived a control from it: "pin test-binary concurrency to 1". **Cargo runs test targets SERIALLY**, one executable at a time, so that concurrency is already 1 and the control pins nothing. Measured in this project's own logs: in `20260831T093655Z-857818/07-sweep.log` the **119 ordinary targets** each report before the next starts — **zero** overlapping starts — and the two trailing result lines (numbers 120 and 121 of 121) are the `Doc-tests` groups, not targets. *An earlier version of this cell said "alternates strictly, 119 to 121", which is the very claim the paragraph below retracts.* `--test-threads=1` is a *different* knob — it serializes test functions **within** one executable — and does not stand in for the control either. **The real difference between the steps is which binaries run and how long the whole step takes, not how many run at once.** A replacement control has to be designed; this row no longer has one | | **relation to U2 — a NEAR MISS, do not match it there** | the PTY fragment is U2's exact family (`stty -a output was: ""`), but U2's selector field names only `m6_1_pty_raw_mode_disables_kernel_echo`. U2's occurrence 2 saw raw **and** canonical fail together; here **canonical redded alone and raw passed**, which U2's evidence has never shown. It is recorded here rather than folded into U2 so that the "canonical alone" case stays visible | | **relation to U6 — its own instruction, honoured** | `composition_overhead_under_ten_percent` is one of U6's two selectors, and U6 says plainly: "If a future run reds **one** of these without the other, that is a different incident and should be judged as one." It redded without `criterion_1_end_of_line_typing…`, in a different step, at a far larger margin (1.613× here against U6's 1.297×). Judged as a different incident, as instructed | | **what this row does NOT assert** | that the two selectors share a mechanism. They failed together once; they belong to different subsystems; and U7 already refused this exact merge for U6. The **co-failure inside one step with an in-run green control** is the signature — not either name, and not a shared cause | @@ -1042,15 +1259,54 @@ green in the other run**. | **relation to U6** | run B's selector is one of U6's two, redding **without** `composition_overhead_under_ten_percent`. U6 instructs that one-without-the-other is a different incident; honoured here | | **what this row does NOT assert** | a shared mechanism between the two rows, or any mechanism at all. **The signature is the rotation across an identical commit** — not either name | -**Why this family keeps recurring, stated plainly.** Every row in it is -a wall-clock budget asserted **inside a workspace-wide parallel test -run**. `cargo test --workspace` starts many test binaries at once, so -each budget competes with the rest of the sweep in **every** run, -including the ones that pass. A 4.5% overshoot on a 1ms budget is not a -signal about the code. **U9 already named the discriminating control** -— pin test-binary concurrency to 1 and separately load a lone `--lib` -binary — and it remains unrun. Until it runs, this family should not -consume another review round. +**Why this family keeps recurring — with its stated premise CORRECTED, +because it was false.** Every row in it is a wall-clock budget asserted +inside a workspace-wide test run. This paragraph used to add that +"`cargo test --workspace` starts many test binaries at once". **It does +not. Cargo runs test targets SERIALLY, one executable at a time**, and +this project's own gate logs measure it. In +`20260831T093655Z-857818/07-sweep.log` (and reproduced on +`20260831T130742Z-2805186`): **119 ordinary targets, each of which +reports its result before the next one starts** — zero cases of one +`Running` line following another. There are **121 result lines in total**, and the last two — +**numbers 120 and 121** — are not targets: they belong to the doc-test +groups `Doc-tests pmacs` and `Doc-tests pmacs_protocol`, which cargo +labels differently and runs after everything else. So the alternation is +119 `Running`/result pairs, then two doc-test results. + +**Two earlier versions of this paragraph got the arithmetic wrong**, +which is worth leaving on the record in a file about not trusting +unverified numbers: the first called the whole thing "strictly +`RTRTRT…`" with 119 and 121, which cannot be strict; the second called +the doc-test results the 121st and 122nd, when there is no 122nd. + +So the budgets do **not** compete with the rest of the sweep in the way +this family assumed. + +**And the first replacement for that premise did not describe U9 +either.** It said a budget "runs at an arbitrary point in a multi-minute +step". **U9's two selectors are both in the root lib target** — +`m6_1_pty_canonical_mode_keeps_kernel_echo` (`src/process.rs:3967`) and +`composition_overhead_under_ten_percent` (`src/editor.rs:9717`) — and +**note the selector**: U9's row names the CANONICAL test, not +`m6_1_pty_raw_mode_disables_kernel_echo` (`:3945`), and an earlier +version of this paragraph named the raw one. U9's own "relation to U2" +cell turns on exactly that distinction — canonical redded alone while +raw passed — so getting it backwards would have undercut the row it was +trying to correct. +the sweep runs that target **first**, finishing it in about 12 seconds +of a multi-minute step. The sweep's later minutes cannot reach them. + +What survives is narrower still: **the sweep re-runs the lib target late +in the overall gate invocation**, after `03-lib` and `04-lib-crdt` have +already run it, under machine state nobody measured. A 4.5% overshoot on +a 1ms budget remains not a signal about the code. + +**And U9's named control does not discriminate what it claimed** — +"pin test-binary concurrency to 1" pins something that is *already* 1. +See the correction on U9 itself. This family still should not consume +another review round, but it now needs a control someone has to design, +not one already written down. **Widening a budget is not the fix**, and R1 already rejected it. @@ -1110,11 +1366,424 @@ PTY selector. | **relation to U6** | its selector, alone again, in U6's own step. U6's instruction to judge that separately is honoured for the second time — see U9, which did the same | | **relation to U9** | the same budget-plus-PTY co-failure, in `04-lib-crdt` rather than `11-sweep`, with `setsid_escapee…` where U9 had `m6_1_pty_raw_mode…` | -**This family has now produced U6, U9, U10 and U12, and the -discriminating control U9 named is STILL UNRUN**: pin test-binary -concurrency to 1, and separately load a lone `--lib` binary. Four -incidents is enough evidence that the family will keep costing review -rounds until someone runs it. +**This family has now produced U6, U9, U10 and U12 — and the control +U9 named no longer exists to run.** It had two halves. *Pin test-binary +concurrency to 1* is **VOID**: cargo already runs targets serially, so +it pins nothing (see U9). *Separately load a lone `--lib` binary under +synthetic load* survives as an experiment but is **not a discriminator** +— with concurrency fixed at 1 there is no second arm to compare against, +so a red would show load is **sufficient** to produce one, and a green +would show nothing at all. **It could never "clear" load**, and this +file's own rerun rule says why. + +Four incidents is enough evidence that the family will keep costing +review rounds. What it needs is a control someone designs, not the one +written down. + +### U14 — FOUR selectors red in ONE gate run, across three stages + +Recorded on the CRDT identity-undo lane, 2026-08-30, local (Linux), +`scripts/gate` log `20260830T171941Z-3509751`. **The co-occurrence is +the signature**, as it is for U6, U9 and U12: four selectors in **four +unrelated subsystems** — the async runtime, the optimistic-echo +orchestrator, editor composition, and the LSP dispatch seam — failing in +one run is less likely than one loaded machine, and no single selector +reds twice within the run. **U6's own row treats its two selectors as +unrelated subsystems**, so the `04-lib-crdt` pair is two of the four +here, not one. + +| field | value | +|---|---| +| **selectors** | **four.** `03-lib`: `async_runtime::tests::grep_supersede_cancels_predecessor_within_50ms`. `04-lib-crdt`: `optimistic::tests::criterion_1_end_of_line_typing_completes_sub_frame_per_keystroke` **and** `editor::tests::composition_overhead_under_ten_percent` (U6's pair — see below). `07-sweep`: `lsp_dispatch_seams_acceptance::acc34_purge_reaches_a_server_that_is_in_no_attachment` | +| **job / flavor** | local (Linux), one `scripts/gate` run, three different steps | +| **required fragments** | `grep supersede did not cancel within 50ms` + an `elapsed:` value; `criterion 1: per-keystroke orchestrator time` + `exceeds 1ms`; `composition machinery added more than 10% overhead`; `is not ready for requests (state: initializing)` | +| **status** | **new incident, ONE occurrence** | +| **what IS established** | all four fragments captured from the durable stage logs. Margins: `52.44341ms` against 50ms (4.9% over); `1.689259ms` against 1ms; `1.592×` against 1.10×. The `07-sweep` failure is **not** a budget — an LSP server was asked for a request while still `initializing` | +| **what is NOT** | any shared mechanism, and **any load measurement**: no `/proc/loadavg` reading was taken during or after this run. Three stages, **four subsystems**, and one of the four selectors is a readiness race rather than a clock | +| **the observing tree** | the lane's revision-5 commits: an enumeration in a `#[cfg(test)]` predicate and documentation. It touches `async_runtime`, `optimistic`, `editor` and the LSP dispatch seam **not at all** | + +**An earlier version of this row claimed a SECOND occurrence, and that +was a matching-rule violation.** The run 40 minutes later +(`20260830T175657Z-3881334`) redded a **different selector set** — +`full_buffer_summary_flatten_scales_on_large_grammar_file` and +`dired_renders_10k_entries_within_200ms` in place of +`grep_supersede…` and `acc34_purge…`. Under "How a row matches" above, +the exact selectors must match; a rotated set is a **new incident**. +It is now **U15**, and the `04-lib-crdt` pair the two runs do share is +recorded where it belongs, as U6's own occurrence. + +**Relation to R1, and it is NOT a match.** The `03-lib` failure carries +R1's required fragment `supersede did not cancel within 50ms`, but R1's +selector is `supersede_cancels_in_flight_job_within_50ms` and this is +`grep_supersede_cancels_predecessor_within_50ms` — **a different test**. +This registry matches on selector *and* fragments, and U6's own +instruction ("one without the other is a different incident") points the +same way. Recorded as a sibling, not an occurrence. + +**One thing the sibling shows for free, and R1 should have it.** R1's +row records that its assertion "still omits its measurement — +`started.elapsed()` is in hand at the panic and the message reports none +of it, so this occurrence's margin is as unrecoverable as every prior +one's." **The sibling test already reports it**: `(elapsed: +52.44341ms)`, which is how the 4.9% margin above is known at all. The +measurement-design question R1 defers to the async-runtime lane is +untouched by this — but the cheap half of it is demonstrably already +written, next door in the same module. + +**Reruns: all four selectors green in isolation** — `grep_supersede…` +`1 passed`, U6's two `1 passed` each, and the whole +`lsp_dispatch_seams_acceptance` binary `15 passed`. Per this file's +rerun rule that establishes **intermittence only**; it exonerates +nothing, and in particular it does not show the tree is innocent, only +that the failures do not reproduce alone. + +### U15 — a rotated multi-red cluster, with a contemporaneous load reading + +Recorded on the CRDT identity-undo lane, 2026-08-30, local (Linux), +`scripts/gate` log `20260830T175657Z-3881334` — 40 minutes after U14's +run, on the same tree. **A new incident rather than a U14 occurrence**, +because the selectors rotated and this file matches on the exact set. + +| field | value | +|---|---| +| **selectors** | `03-lib` (**default features**): `editor::tests::composition_overhead_under_ten_percent` **and** `semantic_render::tests::full_buffer_summary_flatten_scales_on_large_grammar_file`. `07-sweep`: `dired_acceptance::dired_renders_10k_entries_within_200ms` | +| **job / flavor** | local (Linux), one `scripts/gate` run, steps `03-lib` and `07-sweep` | +| **required fragments** | `composition machinery added more than 10% overhead`; `full-buffer flatten took` + `the event sweep must stay ~linear`; `10K entries must render within 200ms; took ` | +| **status** | **new incident, one occurrence** | +| **what IS established** | margins `1.450×` against 1.10×, `1.274901136s` against a ~linear expectation, and `221.459827ms` against 200ms (10.7% over). **`/proc/loadavg` read immediately after the run: `34.04 38.45 26.29`**, with the CPU saturated by unrelated `lean` processes — nothing this session started, and no `cargo`, `rustc` or `scripts/gate` process of mine left running | +| **what is NOT** | that the load caused any of it. See below | + +**Two selectors here belong to other rows and are deliberately NOT +claimed as their occurrences.** `composition_overhead_under_ten_percent` +is one of U6's two, and it redded in `03-lib` **without** its partner +and under **default features**, not U6's `04-lib-crdt`/`crdt` flavor — +U6's own instruction is that one-without-the-other is a different +incident. `dired_renders_10k_entries_within_200ms` is one of U7's three, +and this is the **second** time that same selector has redded — U7's own +instruction is that a repeat of one selector is a different incident. +Both instructions are honoured rather than quoted and ignored. + +*(The same run's `04-lib-crdt` step redded U6's pair together, in U6's +flavor and step. That IS a U6 occurrence and is recorded there.)* + +**What the load number establishes, stated at its real strength — and +it is NOT this registry's first.** **U7 has carried a load average since +2026-08-09** (`12.9 / 23.9`, in its job/flavor field); what U7 records as +unmeasured is something narrower, whether the shared +`CARGO_TARGET_DIR` and its sibling worktree builds *produced* that load. +An earlier version of this block said U6 and U7 had both wanted a number +since 2026-08-09. Half of that was wrong. + +What `34.04` is: **the first contemporaneous load reading for a U6 +occurrence** — U6's row has said since 2026-08-09 that its confound "was +not measured" — and **a new reading alongside a recurring U7 selector**, +not U7's first. That makes load a **measured confound present +contemporaneously with a multi-red run**. It does not make it the cause, +and three specific things stop it short: + +* **the reading is a single point, taken after the fact.** No + `/proc/loadavg` was captured during U14's run or the two out-of-gate + runs, so there is no series to correlate margins against; +* **the margins are not monotonic.** + `composition_overhead_under_ten_percent` went `1.182×` → `1.592×` → + `1.527×` across the three runs, and in this run alone it reports + `1.450×` in `03-lib` and `1.527×` in `04-lib-crdt`. Only + `criterion_1`'s three points rise monotonically (`1.343883ms`, + `1.689259ms`, `2.269247ms`), and three points with one load reading + is not a dose-response; +* **an earlier version of this write-up said "a load average of 34 + explains it without any help."** That overstates it. Severe unrelated + load was present; whether it produced these particular margins is + unmeasured. + +**What it does change:** for U6, "one loaded machine" stops being a +hypothesis offered in good faith and becomes a **quantity on the +record** — a second data point for the correlation U7's reading started. +It retires nothing: **U15 itself**, U6, U7, U14 and R1 all keep their +dispositions, and the budgets remain wall-clock assertions whose +measurement design nobody has defended. + +**Reruns: green in isolation** — `composition_overhead…` `1 passed`, +and the `dired_acceptance` selector had already been shown green in +isolation by U7. Intermittence only, per the rerun rule. + +### U16 — a git invocation finds its working directory deleted + +Recorded on the CRDT identity-undo lane, 2026-08-31, local (Linux), +`scripts/gate` log `20260831T083021Z-272257`, step `07-sweep`, **and +again the same day** — see the second occurrence below. **Not a budget +row** — nothing here is a clock. It is the only row in this file that +arrives with a **named candidate mechanism inside the test suite**, +which is why it was worth recording before it had reproduced. + +| field | value | +|---|---| +| **selector** | `--lib packages::fetcher::tests::cache_survives_across_fetcher_instances` | +| **job / flavor** | local (Linux), `scripts/gate` step `07-sweep` (`cargo test --workspace --no-fail-fast`) | +| **required fragments** | `Unable to read current working directory: No such file or directory` + `remote did not send all necessary objects` | +| **status** | **SECOND OCCURRENCE 2026-08-31 — it reproduced, in the same step, with the same fragments** | +| **what IS established** | the fragments, captured from the durable stage log at `src/packages/fetcher.rs:929`. `1989 passed; 1 failed`. The test spawns `git` against a `file://` remote in a temp dir | +| **what is NOT** | that the mechanism below is what happened. It is a candidate with a citation, not a demonstrated chain | +| **the observing tree** | the lane's docs-only commit. It touches `src/packages/` not at all | + +**The candidate mechanism, and the load-bearing step is CHILD +INHERITANCE.** An earlier version of this row stopped at "the window +exists", which misses why restoring the cwd does not close it: + +1. `src/file_io.rs:434` — `bare_filename_saves_in_cwd` — calls + `std::env::set_current_dir(dir.path())`, mutating **process-global** + state and pointing it at a `TempDir`; +2. concurrently, `cache_survives_across_fetcher_instances` reaches + `f1.fetch(&url)` (`fetcher.rs:929`), which clones via `run_git` + (`:305`). `run_git` calls `run_git_inner(None, …)`, and + `run_git_inner` (`:322`) sets `cmd.current_dir` **only when `cwd` is + `Some`** — `if let Some(d) = cwd { cmd.current_dir(d); }`, + `:329`–`:330`. With `None`, **the spawned `git` INHERITS the + parent's cwd** — the temp directory; +3. the parent then restores its own cwd. **That does nothing for the + child**, which already has its working directory; +4. the `TempDir` drops. `git` is now a live process whose cwd is a + **deleted directory**, and `fatal: Unable to read current working + directory` is exactly what that produces. + +So the restore in `bare_filename_saves_in_cwd` is not merely +insufficiently early — it is **irrelevant to the child**, which is why +care inside that one test cannot close this. + +**What would settle it**, and neither has been run: + +* run the two selectors concurrently in a tight loop until the failure + reproduces, which converts the candidate into a demonstration; +* or make the hazard structural rather than probabilistic. **A serial + guard around `set_current_dir` tests is NOT one of the options**, and + an earlier version of this row offered it: the child outlives the + guard, so any unguarded test that spawns a process inheriting the cwd + is still exposed. What does work: + * **remove the process-global mutation** — `bare_filename_saves_in_cwd` + exists to check that a bare filename resolves against the cwd, and + `save_atomic` could take the directory rather than inheriting it; + * **run that test in a subprocess**, so its cwd is its own; + * **serialize the whole lib-test binary** (`--test-threads=1`), which + removes the concurrency the race needs — at the cost of the whole + binary's wall clock, and note U17, where that same flag is a + candidate for causing a different failure. + +**Reruns: green in three isolated runs of the selector, and in EIGHT +full parallel `cargo test --lib` runs** (1990 passed each). Per this +file's rerun rule that establishes **intermittence, and nothing more**. +An earlier version added "and here it also says the window is narrow" — +**it does not**. Non-reproduction over eight runs says the failure did +not recur in eight runs. It says nothing about the width of *this +candidate's* window, which no measurement here has sized. + +**SECOND OCCURRENCE, 2026-08-31, log `20260831T174104Z-3438184`, step +`07-sweep`** — same selector, same panic site `fetcher.rs:929`, both +required fragments, `1988 passed; 2 failed`. **This is the first time +the row has reproduced, and it settles the point above in the right +direction**: withdrawing "the window is narrow" was correct, because +eight green runs had not measured it, and the failure returned within +the day. The candidate mechanism in §"child inheritance" above is +unchanged and still a candidate — nothing in this occurrence +demonstrates the chain either. + +**And it passed again immediately after**, in all three stages of the +next gate run — `03-lib`, `04-lib-crdt` and the same `07-sweep` context +— at `ea786a2`, log `20260831T174716Z-3535694`. Two reds and many greens +**all on 2026-08-31**: **intermittent**, at a rate nothing here has +measured. *(An earlier version said "across two days"; every run cited +by this row is the same day, as the sentences above it already said.)* + +**Not folded into U14 or U15.** Different selector, different fragments, +different step, and a different kind of failure: those are wall-clock +budgets under load, this is a race over process-global state. U14's +`acc34_purge` readiness failure is the nearest relative in kind, and even +that is a different mechanism. + +### U17 — a supersede race lost the OTHER way, on `main`, single-threaded + +Surfaced 2026-08-31 by the **merge-base control dispatched for R6's +second occurrence** — so it is a red on `main` at `aae5b35`, on no +branch at all. + +**An earlier version said "no PR run can show this." That is wrong**: a +PR run exercises the same test and could fail it identically. What only +a `main`-side run can establish is that it fails **on `main`** — that +there is no observing branch to suspect — and that is the distinction +the dispatch actually bought. + +| field | value | +|---|---| +| **selector** | `--test m8_1_acceptance read_dir_supersede_cancels_in_flight_predecessor` | +| **job / flavor** | GitHub Actions, `Test (crdt)`: `cargo test --all-targets --no-default-features --features luajit,crdt -- --test-threads=1` | +| **required fragments** | `first read_dir must be superseded; got ok` | +| **status** | **new incident, one occurrence, ON `main`** | +| **what IS established** | `9 passed; 1 failed`, panic at `tests/m8_1_acceptance.rs:278`, [run 33375945966](https://github.com/levineuwirth/pmacs/actions/runs/33375945966) job 99437344558, head `aae5b35` | +| **what is NOT** | any mechanism. The candidate below is a reading of the assertion, not a diagnosis | +| **attribution** | **none available, and none needed** — `aae5b35` is `main`. There is no observing branch to suspect | + +**It fails the OPPOSITE way to R1 and R5, and that is the interesting +part.** Both of those are **deadline** failures — a cancellation that +did not arrive in time (`supersede did not cancel within 50ms`, `async +pump deadline exceeded`). This one reports `got ok`: the first +`read_dir` **completed successfully** rather than reporting cancellation. + +**What `got ok` proves, precisely:** the predecessor **completed +successfully before the cancellation took effect**. It does **not** +establish when the supersede arrived — an earlier version of this row +said "it arrived after there was nothing left to supersede", which +assumes a late arrival the assertion cannot see. A supersede that +arrived in time and whose cancellation simply did not take effect first +produces the identical message. + +**Candidate mechanism, stated as one — and stated smaller than an +earlier version had it.** The job runs **`--test-threads=1`**, which +serializes the **test functions inside one libtest executable**. A test +that supersedes a job "in flight" depends on the predecessor still being +in flight, and removing sibling test functions from the same process +**removes one source of contention** for it. + +That is all it supports. The earlier wording said the predecessor is at +its *fastest* and the window at its *narrowest*; neither follows. Other +contention remains — the rest of the machine, the CI runner's own load, +and every other process — and nothing here measured the predecessor's +duration with the flag on versus off. It is **not** a diagnosis, and +nothing rules out a real supersede defect. + +**Worth noting for U9 — and NOT as an instance of its control.** An +earlier version said this job "already does" what U9's control asks. It +does not, and the distinction is the whole point of U9's premise: +**`--test-threads=1` serializes test FUNCTIONS within one executable; it +does not pin test-BINARY concurrency.** Those are different knobs. See +the correction recorded against U9 and U12 below, which is larger than +this note. + +**Not R1 and not R5**, on this file's own matching rule: different +selector, different module, different assertion. R5's row draws exactly +this distinction against R1 and the same reasoning applies again — +sharing a subject is not sharing a signature. + +**No rerun was performed.** U3's lesson and R6's "never a green rerun" +disposition both apply, and there is no branch here whose merge this +would gate. + +### U18 — a Go module checksum fetch fails before anything is built + +Recorded on the CRDT identity-undo lane, 2026-08-31, +`Test (ubuntu-latest / luajit)` on PR #246 at `a7c4b3a` +([job 99499800716](https://github.com/levineuwirth/pmacs/actions/runs/33395769472/job/99499800716)). +**A new class for this file: nothing was built and no test ran.** The +job died in its toolchain-setup step. + +| field | value | +|---|---| +| **selector** | none — this is not a test. The failing step is `go install golang.org/x/tools/gopls@v0.16.2`, part of the LSP fixture setup | +| **job / flavor** | GitHub Actions, `Test (ubuntu-latest / luajit)` | +| **required fragments** | `sum.golang.org/tile/` + `stream error` + `INTERNAL_ERROR; received from peer`, while `verifying module: golang.org/x/telemetry` | +| **status** | **new incident, one occurrence** | +| **what IS established** | the failure is a **checksum-database read over HTTP/2**: `reading https://sum.golang.org/tile/8/0/x114/644: stream error: stream ID 41; INTERNAL_ERROR; received from peer`. `gopls@v0.16.2` and the `x/telemetry` pin both downloaded successfully first; only the sum-database verification failed. Job duration 1m45s, exit code 1 | +| **what is NOT** | anything about this repository. **No `cargo` command ran**, no test executed, and the pinned versions are the point — the workflow comments say the pin exists so "CI behaviour" does not "drift with upstream releases" | +| **attribution** | **none to the branch.** The step runs before any pmacs code is compiled, and the other 13 checks passed on the same head | + +**Why it gets a row at all.** Every other row here is a test that +failed. This is **infrastructure the workflow depends on failing to +answer**, and it presents as a red check indistinguishable from a real +one at a glance. A future occurrence should be recognisable as this +rather than investigated as a product defect, which is the whole +purpose of a signature. + +**It is genuinely external, and that is a claim with a limit.** The +fragments name `sum.golang.org` — Google's checksum database — returning +an HTTP/2 stream error. Nothing in this repository can produce that. +What this repository *does* control is whether a transient upstream +outage fails a whole matrix leg, and that is a real question this row +does not answer. + +**An earlier version of this paragraph listed three options, and all +three were wrong.** They are corrected here rather than deleted: + +* **`GONOSUMCHECK` is not a Go environment variable.** It was invented + by that sentence; +* **`GOFLAGS=-mod=mod` does not bypass checksum-database + authentication.** It selects the module *update* mode, which is a + different thing; +* **vendoring does not follow from "a vendored `gopls`".** The step is a + version-suffixed `go install …@v0.16.2`, and that form **ignores + vendor directories**, so pinning that way needs a different + installation path entirely. + +**`GONOSUMDB` was thrown out with them, and should not have been.** It +is real, and `go help environment` on the toolchain in this checkout +documents `GOPRIVATE, GONOPROXY, GONOSUMDB` as glob patterns of module +prefixes "that should always be fetched directly **or that should not be +compared against the checksum database**" — precisely the step that +failed here. So it is **technically applicable**. Whether exempting a +dependency from checksum verification is an acceptable authentication +tradeoff to buy CI stability is a real question, and a different one +from whether the knob exists. The same page notes `GOINSECURE` does +**not** disable that validation, which is the kind of near-miss that +made the original sentence wrong. + +**The options that stand, then:** `GONOSUMDB`/`GOPRIVATE` with the +tradeoff stated, or **retrying the step**. Choosing between them is not +this lane's work. Both directions of this paragraph's history are +recorded because both were errors: **inventing an environment variable +to fill a sentence**, and then **discarding a real one while correcting +it**. + +**No rerun was performed**, and deliberately: U3's lesson is to read the +log first, and the log is now read and quoted above. Whether a rerun +would pass is uninteresting — a transient network error is *expected* to +pass on retry, and doing so would establish nothing while destroying +nothing either. It is left for whoever next pushes to this branch. + +### U19 — a terminal bell is not observed within a 5s poll + +Recorded on the CRDT identity-undo lane, 2026-08-31, local (Linux), +`scripts/gate` step `07-sweep`, log `20260831T174104Z-3438184` — the +**same run** that produced U16's second occurrence, and recorded +separately because the selectors and fragments differ. + +| field | value | +|---|---| +| **selector** | `--lib daemon::tests::terminal_bell_baseline_suppresses_history_and_delivers_each_new_bell_once` | +| **job / flavor** | local (Linux), `scripts/gate` step `07-sweep` (`cargo test --workspace --no-fail-fast`) | +| **required fragments** | `initial terminal bell timed out` | +| **status** | **new incident, one occurrence** | +| **what IS established** | panic at `src/daemon.rs:5296`, `1988 passed; 2 failed`. The assertion is a **5-second poll**: `while bell_count(buffer_id) != Some(1) { tick_processes(); assert!(Instant::now() < deadline); sleep(10ms) }` (`src/daemon.rs:5293`–`:5298`) | +| **what is NOT** | whether the bell never arrived or arrived late. The loop cannot tell those apart, and **the panic reports no elapsed value** — R1's complaint about its own assertion, in a second place | +| **the observing tree** | a documentation-only commit | + +**It is a deadline, but a much slacker one than the budget family's.** +U6, U7, U9, U10, U12 and U15 assert that work finishes *fast* — 1ms, +1.10×, 200ms. This one allows **five seconds** for an event to be +observed at all: 5000× the 1ms budget, but only **25×** the 200ms one. +*(An earlier version said "three orders of magnitude", which does not +hold against the 200ms row it was comparing to.)* Folding this into that +family would still blur a real distinction, but the distinction is one +of degree, not of kind. + +**What the evidence supports is narrower than the earlier wording.** It +shows that **no bell was observed within five seconds** — not that the +bell "never comes", and **not that scheduling cannot explain it**. Five +seconds is a long time on an idle machine and a short one on a loaded +one, and nothing here measured which this was. + +**What it shares with R1 is the missing measurement.** `Instant::now()` +is in hand at the panic and the message reports none of it, so **this** +occurrence's margin is unrecoverable — exactly what R1's row records +about itself, and what U11 cost this project once already. **Adding the +elapsed value would not make a future occurrence comparable to this +one** — that margin is gone for good. It would make future occurrences +comparable **to each other**, which is the whole of what the change buys +and is still worth having. + +**Reruns: it PASSED in all three stages of the next gate run** — `03-lib`, +`04-lib-crdt` and the exact `07-sweep` context it failed in — at +`ea786a2`, log `20260831T174716Z-3535694`. **So it is intermittent.** +An earlier version of this row said "no rerun was performed"; the very +next gate run was one, and the row said otherwise until review caught +it. Per this file's rerun rule, three green runs establish +**intermittence only**. ### U13 — gate prune-reporting row receives empty child stdout in `sweep` diff --git a/docs/crdt-identity-undo-framing.md b/docs/crdt-identity-undo-framing.md new file mode 100644 index 0000000..d922a51 --- /dev/null +++ b/docs/crdt-identity-undo-framing.md @@ -0,0 +1,343 @@ +# Identity-replace undo — a CRDT-version delta is not a text delta + +**Status: revision 5 — APPROVED at revision 4 and IMPLEMENTED** +(PR #246, branch `crdt-identity-undo`). Revision 5 is a correction pass +answering implementation review; it changes the invariant's shape, not +its ruling. + +Revision 5 answers three findings against the implementation: + +1. **the predicate conflated an empty TEXT delta with a version + delta.** It called every empty-range/zero-insertion edit + `version_only` and then accepted `(History, empty, None)` through a + wildcard arm — which contradicts this framing's own "the op must + survive". The rule is now a full enumeration over three independent + axes (§1a), and C5 asserts all four empty-delta quadrants rather + than two; +2. **the public `Edit` doc was factually false**, saying forward + `apply_edit` never produces the empty-delta shape while C2b proves + all three forward empty forms do. The shape is now named an **empty + text delta**, reachable on both paths, with `crdt_op` as the + discriminator; +3. **R7's write-up overstated what the paired gate runs exclude** — see + `docs/ci-red-signatures.md`; the pair excludes the source tree and + nothing else. + +Revision 4 answered review of 3, which found one substantive gap: **C9 +guarded the census by file set and count, which a same-file substitution +walks straight through.** C9 now asserts the exact +`(file, impl target)` pairs, and its claim is scoped to in-tree +implementations. + +Revision 3 answered review of 2 by completing §4's census: it closes by +construction, and two of its results changed the framing's own claims. + +## 1. The decision, ANSWERED — and it is about PROVENANCE, not shape + +**A visible TEXT delta and a CRDT-VERSION delta are INDEPENDENT +dimensions of `Edit`.** An `Edit` may legitimately carry +`crdt_op = Some(_)` with `range.is_empty() && inserted_len == 0`. + +**Revision 2 stated that without qualification, and review 2 showed why +that is too loose:** an `Edit` carries no provenance marker, so if the +shape alone were legitimate the invariant would have nothing left to +assert. The precise answer: + +> **An empty TEXT delta carrying a CRDT op is legitimate when the +> `Edit` came from `undo`/`redo`, and REQUIRED there. On the FORWARD +> path the same shape carrying an op is a bug, and stays asserted.** + +That is a real narrowing, not a repeal, and it is what makes C5 +testable at all. + +### 1a. The three axes, enumerated + +Revision 4 wrote this as one predicate with a default, and the +implementation inherited the gap: `(History, empty delta, None)` fell +through a wildcard and was accepted. The axes are independent — that is +the lane's whole claim — so the rule is a full enumeration: + +| provenance | text delta | `crdt_op` | verdict | +|---|---|---|---| +| forward | empty | `None` | **valid** — a syntactic no-op | +| forward | empty | `Some` | **invalid** — the original bug | +| forward | real | `Some` | valid | +| forward | real | `None` | invalid | +| history | empty | `Some` | **valid** — a version-only edit | +| history | empty | `None` | **invalid** — the version advance is gone | +| history | real | `Some` | valid | +| history | real | `None` | invalid | + +**An empty text delta is a SHAPE, not a verdict.** Both paths reach it. +`crdt_op` is what separates them, and each direction of that separation +is asserted. + +**Why this answer:** + +- The invariant it contradicts was written for `is_no_op_edit` + (`src/buffer.rs:1836`), a **pre-check on the `EditOp`** reached only + from `run_rope_edit_and_broadcast` (`:1256`). **`undo_crdt_mode` and + `redo_crdt_mode` never reach it** — they diff two ropes via + `derive_replacement_edit` (`:1440`, `:1525`) and attach the op + `crdt.undo()` produced (`:1454`), so identical ropes yield an empty + range describing a real operation. +- **Forward edits reach the empty-delta shape routinely** — each of + the three syntactically empty `EditOp` forms produces exactly it, as + C2b asserts. What is unreachable forward is the shape **carrying an + op**: an empty form short-circuits to `(None, None)`, and a + real-delta form is not empty. So forward "empty range and zero + insertion" implies `crdt_op == None`, still — which is what lets the + invariant keep its full strength there. +- The op must survive. Dropping it would lose a version advance the + replicas need — which is what C3 now actually tests, and revision 1's + C3 did not. +- **The codebase already assumes this, in two places written for other + reasons.** `FoldStore::translate` (`src/fold.rs:211`–`:213`) and + `BufferStyleSpanTranslator::on_edit` (`src/overlay.rs:261`–`:263`) + both return early on `old_len == 0 && new_len == 0`, and both say so + in a comment — *"Buffers broadcast no-op edits; nothing moved."* This + lane is not introducing a doctrine; it is naming one that consumers + were already written against. +- The public contract has room for it. `src/rope.rs:301`–`:303` + enumerates pure insert, pure delete and replace, and **has no fourth + case**; the `crdt_op` field doc (`src/rope.rs:316`) goes further and + asserts the conflation outright ("`None` … for no-op edits in CRDT + mode"). Both are updated by this lane. + +**The empty range's LOCATION is settled by §4's census, not deferred.** +It stays at the buffer end. No consumer is harmed there, and for the +one consumer whose cost depends on it, the buffer end is the *cheapest* +choice — see §4. + +## 2. What is already known — and precisely how well + +`src/buffer.rs:3044` carries a deterministic fixture, +`crdt_undo_of_an_identity_replace_reports_a_no_op_edit_carrying_an_op`, +`#[ignore]`d at `:3042` and documented from `:3005`. It reduces this +exact case: replacing bytes with **identical bytes** is a textual no-op +but a real CRDT delete-plus-insert, so undoing it advances the CRDT +version while leaving text unchanged. + +**Its evidence is narrower than revision 1 claimed, and narrower in one +more place than revision 2 admitted:** + +| claim | how it is established | strength | +|---|---|---| +| content stays correct | **asserted** in the fixture — rope and CRDT projection agree | direct | +| the op reaches broadcast consumers | **by INSPECTION of the call sites** | reasoning, not execution | +| the cursor does not jump | **by INSPECTION** — `EditorCore::undo` only clamps to length | reasoning, not execution | + +**The cursor row was marked "direct" in revision 2. It is not.** The +fixture body (`buffer.rs:3044`–`:3093`) contains **no reference to +`EditorCore` and none to a cursor**; it exercises `Buffer` alone. The +cursor claim is inspection of a different function than the one the +fixture runs. + +**Nothing here replays the op on a remote replica or witnesses +convergence.** Revision 1 said "replicas stay converged" as though it +were established. It is not. **That is exactly what C3 must newly +establish**, and it is the main new evidence this lane produces. + +The CI red that prompted the lane is a randomly sampled recurrence of +this fixture, not a new defect. + +## 3. Terminology, because revision 1's contradicted itself + +**An identity replace IS a forward textual no-op**, and it *does* +produce an operation. So "forward textual no-ops produce no operation" +is false, and revision 1 asserted it while §2 said the opposite. + +The correct statement names a **syntactic** category: + +> The three **syntactically empty `EditOp` forms** — `Insert` with +> empty bytes, `Delete` with an empty range, and `Replace` with both +> empty — produce no CRDT operation. + +That is what `is_no_op_edit` tests, and it stays true. + +## 4. The consumer census — COMPLETE + +Revision 2 listed `broadcast_on_edit` as a row reading *"every attached +view — not enumerated, owes."* **That is a dispatcher, not a consumer, +and review 2 was right that it cannot stand.** Here is the enumeration. + +Both `undo_crdt_mode` (`buffer.rs:1456`) and `redo_crdt_mode` +(`:1537`) do broadcast, so this path is real. + +### 4a. How the census closes + +Three measurements bound it, so it is complete by construction rather +than by search effort — **for this tree**; see §4d on why no in-tree +measurement can reach further: + +1. **The `View` trait's `on_edit` default is `Ok(())`** + (`src/view.rs:450`–`:452`). Every impl that does not override it is + **structurally inert** — it never reads the range. +2. **Exactly four non-test impls override `on_edit`**: `ParseView` + (`syntax.rs:1637`), `TextView` (`text_view.rs:521`), + `FoldStoreTranslator` (`fold.rs:274`), `BufferStyleSpanTranslator` + (`overlay.rs:248`). The other twelve inherit the default. +3. **Exactly four production `Buffer::attach_view` call sites exist** + — `fold.rs:341`, `lua_bindings/mod.rs:3963`, `:4008`, `:8137`. + Measured over the 50 occurrences of `attach_view` outside its own + definition: **38 sit inside `#[cfg(test)]`**, and of the 12 + remaining, **8 are doc comments or a different API** (the Lua + `pmacs.diag._attach_view` name, and `SyntaxRegistry::attach_view` at + `lua/mod.rs:8138`, which registers a handle rather than a buffer + view). + +### 4b. Broadcast consumers, classified + +| attached view | site | reads range? | verdict | +|---|---|---|---| +| `FoldStoreTranslator` | `fold.rs:341` | via `FoldStore::translate` | **INERT** — explicit `old_len == 0 && new_len == 0` early return at `fold.rs:211`–`:213` | +| `BufferStyleSpanTranslator` | `lua/mod.rs:4008` | yes | **INERT** — same explicit early return, `overlay.rs:261`–`:263` | +| `ParseView` | `lua/mod.rs:8137` | yes | **PERMITTED, justified below** | +| `LuaInterceptView` | `lua/mod.rs:3963` | — | **INERT** — overrides `intercept_edit` only (`lua/mod.rs:2132`); inherits the `Ok(())` default | + +**`ParseView` is the one permitted effect.** At 0→0 its splice +(`syntax.rs:1656`) is `source.splice(n..n, [])` — the source mirror is +**unchanged** — and it pushes one `InputEdit` with +`start_byte == old_end_byte == new_end_byte` and all three `Point`s +equal (`:1661`–`:1668`). **Why that is acceptable:** a degenerate +`InputEdit` describes no change, so the incremental parse it feeds must +produce an identical tree. **C4 asserts that rather than assuming it**, +and also asserts the pending queue drains, since an effect that +accumulates per undo would not be acceptable. + +### 4c. Direct (non-broadcast) consumers + +**Revision 2 filed `TextView` under broadcast. It is not attached to +any buffer** — it lives on the window (`win.text_view`) and +`EditorCore::undo` calls it directly at `editor_core.rs:2846`. + +| consumer | reads range? | verdict | +|---|---|---| +| `Buffer::adjust_marks_for_edit` (def. `buffer.rs:1609`; called `:1442` undo, `:1527` redo) | yes | **INERT, arithmetically** — with `start == end` and `inserted_len == 0`, every branch is identity: `pos < start` → `pos`; `pos > end` → `pos - 0 + 0`; `pos == start` → `start` under both gravities (`:1617`–`:1629`) | +| `EditorCore::undo` → `TextView::on_edit` (`editor_core.rs:2846`, body `text_view.rs:521`) | yes | **PERMITTED** — `rebuild_lines_from(buf, line_at_offset(range.start))`. Text is unchanged, so the rebuild is **output-identical**; the cost is the tail of the buffer from `range.start`. **The buffer-end location makes this the CHEAPEST possible rebuild** — moving the range to the edit site would rebuild strictly more | +| `search_invalidate_for_edit` → `mark_stale` (`editor_core.rs:1974`) | **no** | **PERMITTED** — unconditional and range-independent. Search matches are marked stale on an edit that changed no text. Acceptable (correctness is preserved; a re-search is redundant, not wrong), and **moving the range would not change it** | +| `search_invalidate_for_edit` → `translate_search_origin` (`editor_core.rs:1984`) | yes | **INERT, arithmetically** — with `start == end` and `inserted_len == 0`: `pos < start` → `pos`; `pos > end` → `pos - 0 + 0`; else `start + 0`, reachable only at `pos == start` (`:1994`–`:2000`) | + +### 4d. The disposition + +**Five inert, three permitted, none harmed. The range does not move,** +and that conclusion now rests on measurement rather than on a deferral. + +| | inert | permitted | +|---|---|---| +| broadcast (§4b) | `FoldStoreTranslator`, `BufferStyleSpanTranslator`, `LuaInterceptView` | `ParseView` | +| direct (§4c) | `adjust_marks_for_edit`, `translate_search_origin` | `TextView`, `mark_stale` | + +*(Revision 3 said four and three. Miscount, corrected.)* + +The two permitted effects with a cost — `TextView`'s rebuild and +`mark_stale` — are both **strictly cheaper or equal at the buffer end** +than at the edit site, so the location the fixture called arbitrary is +not merely harmless but weakly preferable. + +**This census is a point-in-time measurement of THIS TREE**, valid at +the commit the lane branches from. Both `View` (`src/view.rs:419`) and +`Buffer::attach_view` (`src/buffer.rs:674`) are **public**, so a +downstream crate may implement `on_edit` and attach it, and no in-tree +measurement can enumerate that. The census, and C9 with it, are scoped +to in-tree implementations; the public contract §5's C7 updates is what +speaks to anyone outside. Revision 3 claimed C4 would guard it against a new +override or attach site; **it cannot — executing three consumers says +nothing about a fourth, and that claim is withdrawn.** C9 is the guard +that actually holds, and it holds the one condition that matters: if +the set of `on_edit` overrides is unchanged, then every attach site, +new or old, attaches a view that is either the inert trait default or +one of the four already classified. + +## 5. Acceptance + +| # | contract | witness | mutation | +|---|---|---|---| +| C1 | the fixture runs, and is not silently re-ignored | un-ignore it; **plus a structural assertion** that no `#[ignore]` attribute precedes the fixture's `fn` (via `include_str!` on the file), **plus** the run's `1 passed; 0 ignored` line recorded as gate evidence | restore `#[ignore]` → the structural assertion fires **and** the recorded line reads `0 passed; 1 ignored`. Without one of these, re-ignoring is a green suite | +| C2a | `is_no_op_edit` classifies all three **syntactically empty forms** as no-ops | assert `is_no_op_edit` **directly** for `Insert{bytes:[]}`, `Delete{range:empty}`, `Replace{range:empty,bytes:[]}` | flip **any one** arm (`buffer.rs:1838`–`:1840`) → C2a fires. Nothing sits between the assertion and the classifier, so this mutant **cannot be masked** | +| C2b | end-to-end, each empty form still yields `crdt_op == None` | apply each form through `apply_edit` on a CRDT buffer | **compound mutant, and it must be**: flip the arm **and delete that variant's defensive early return** — `buffer.rs:1177`–`:1182` (Insert) or `:1192`–`:1194` (Delete). See below | +| C3 | an empty-text history op **replays convergently on a REMOTE replica**, **for both `undo` and `redo`** | seed replica B with the **forward** ops, apply the history op to B, assert **(a)** identical materialized text **and (b)** identical CRDT version/frontier; then apply a **causally dependent** op and assert both still agree | **drop the history op before replay** → text still matches, so only the version/frontier assertion catches it | +| C4a | the history edit is **broadcast at all**, for both `undo` and `redo` | attach a counting view (the `RecorderView` shape, `buffer.rs:2218`) and assert **exactly one** `on_edit` per history op | **delete `self.broadcast_on_edit(&inverse_edit)?`** at `buffer.rs:1456` (undo) or `:1537` (redo) → the count is 0 → C4a fires | +| C4b | the classified consumers are unchanged by the real history edit | attach `FoldStoreTranslator`, `BufferStyleSpanTranslator` and `ParseView`; run the identity-replace op; assert fold store unchanged, span vector unchanged, **parse tree identical**, and `pending_edit_count()` returns to 0 after the drain (`syntax.rs:712`, `:737`) | see the note below — **C4b claims no guard mutation**, and C4a is what makes it non-vacuous | +| C4c | the style-span guard's own contract, pinned where it can fire | call `BufferStyleSpanTranslator::on_edit` with a **synthetic INTERIOR 0→0 `Edit`** whose position falls strictly inside an existing span, and assert the span vector is **byte-identical** — not merely equal in coverage | delete `overlay.rs:261`–`:263` → the span splits into two adjacent fragments and the vector differs → C4c fires | +| C5 | the invariant is keyed on **provenance**, and covers **all four** empty-text-delta quadrants of §1a | preserve the `GenOp` classification (`buffer.rs:3101`, where `op` is moved before it can be classified) as an operation class; extract the shape check to take `(class, &Edit)`; then **inject** all four: `(Forward, empty, None)` accepted, `(Forward, empty, Some)` rejected, `(History, empty, Some)` accepted, `(History, empty, None)` rejected | widen the forward rule → C5 fires; accept `(History, empty, None)` → C5 fires, and **revision 4's two-assertion C5 did not**. **The proptest alone catches neither**, because no generated input reaches either row — which is why C5 is a directed injection, not a property | +| C6 | the executed history-case set **is** `{Undo, Redo}` | after the parameterized loop, assert the collected set of cases actually run equals the literal `{Undo, Redo}`; a `match` over the case enum keeps a future variant from being added silently | drop `Redo` from the case list → the **set assertion** fires. Without it the suite simply runs one case and stays green, which is why revision 3's C6 was a zero-execution witness | +| C7 | the public contract names the **empty text delta** and says which path produces which `crdt_op` | the `Edit` doc (from `src/rope.rs:292`) gains the empty-delta shape **as reachable on BOTH paths** — `None` forward, `Some` from history — and the `crdt_op` field doc stops asserting that no-op edits have no op | leave the doc → it contradicts the code the lane just blessed. **An earlier version of this row said forward `apply_edit` never produces the shape; C2b proves all three forward empty forms do**, so the doc it produced was false and is corrected in revision 5 | +| C8 | **the fixture's own doc comment is corrected**, not just its attribute | rewrite `buffer.rs:3005`–`:3040`: convergence is **established by C3**, not "verified" (`:3023`–`:3026`); the buffer-end range is **ruled and weakly preferable** per §4, not "genuinely arbitrary" (`:3034`–`:3036`); "**The open question**" (`:3030`) becomes the ruling; and the `#[ignore]` reason string (`:3042`–`:3043`) goes with the attribute | leave the comment → the repository's most-read record of this defect still says the decision is open and that convergence was already checked, contradicting §1, §2 and C3 | +| C9 | §4's census stays closed **for in-tree implementations** | walk `CARGO_MANIFEST_DIR/src` and assert the set of **`(file, impl target)` pairs** carrying a non-`#[cfg(test)]` `fn on_edit` override is exactly `{(syntax.rs, ParseView), (text_view.rs, TextView), (fold.rs, FoldStoreTranslator), (overlay.rs, BufferStyleSpanTranslator)}` — pairs, not file set and count, and by name rather than line number | **replace `ParseView`'s override with an unclassified type in the SAME file** → file set and count are both unchanged, and only the pair set catches it. Adding a fifth override anywhere under `src/` fires it too, naming the file and the type | + +**Why C4 was rebuilt.** Revision 3's C4 claimed that deleting the fold +or style guard would make an assertion fire. **Both mutants survive**, +and the arithmetic says why: the history edit sits at the buffer end, +so with `old_start == old_end == len` and `old_len == new_len == 0`, +`BufferStyleSpanTranslator` emits a left fragment `[s, min(e, len))` +for every span within the buffer and no right fragment — the vector is +unchanged with or without the guard (`overlay.rs:269`–`:285`). The fold +store's remaining arithmetic is identity for the same reason. **At this +location the fold guard is an optimization, not a behaviour +discriminator, and no mutation is claimed for it.** What discriminates +is whether the broadcast happens at all (C4a) and whether the style +guard holds where the fragmenting is reachable (C4c). + +*(A span of zero width at exactly `len` would be dropped without the +guard and kept with it. That is not used as a witness: whether such a +span is constructible is unestablished, and a witness resting on a +degenerate value is a worse instrument than the interior injection.)* + +**C2b's mutant is compound because two of three variants mask it, and +the asymmetry is measured.** `apply_to_crdt_then_normalize_bytes` +returns `(None, None)` early for an empty `Insert` (`:1177`–`:1182`) +and an empty `Delete` (`:1192`–`:1194`), so flipping those arms alone +still yields `crdt_op == None` and the simple mutant survives. The +empty `Replace` has **no** early return — `:1208` skips the delete, +`:1211` skips the insert, and control falls through to the +unconditional `Some(crdt_op)` at `:1231`–`:1236` — so there, and only +there, the simple mutant dies. C2a exists so this asymmetry cannot hide +a classifier regression. + +**C3's mutation is the point of C3.** Revision 1's version asserted +text equality alone, and an identity-replace history op leaves text +unchanged — so **dropping the op passed it**. Version/frontier equality +is what discriminates; the causally dependent op is corroboration on +top. + +**C5 is the second half of §1's answer.** §1 makes the shape legitimate +*for history ops*; without a provenance-keyed check there is no +remaining assertion for the forward path, and the invariant would have +been repealed rather than narrowed. + +**C1, C6 and C9 exist because a green suite is not evidence that a +suite RAN.** Re-ignoring a fixture, dropping a parameter, and adding an +unclassified consumer are all silent under ordinary assertions. Each +gets a witness that fails on absence rather than reporting it. + +## 6. Coherence impact (`COHERENCE.md` §20) + +Under the resolution the census confirms — invariant narrowed to +provenance, behaviour unchanged: + +- **Journey steps touched: NONE.** No product behaviour changes; the + work is a test contract, a census, and public documentation. +- **Interaction islands: none added.** +- **Config registry: no entry.** +- **Background work: none started.** + +Revision 2 made this section conditional on a census that had not run. +**It has now run, and no consumer is harmed, so the section is +unconditional.** + +## 7. What this does NOT do + +- **It does not commit the proptest regression seed.** That duplicates + a deterministic fixture and would make a disputed assertion fail + permanently rather than occasionally. +- **It does not re-verify content correctness**, which §2 records as + directly asserted. It *does* newly establish remote convergence, + which §2 records as only inspected. +- **It does not move the empty range**, and after §4 that is a measured + result rather than a deferral. +- **It does not audit `intercept_edit`**, a different stage with a + different contract. The census covers `on_edit` and the direct + consumers of the history `Edit`. +- **It does not reorder the roadmap.** GUI arc 1b remains the next + product lane. diff --git a/src/buffer.rs b/src/buffer.rs index 3ccb5e9..cc2af91 100644 --- a/src/buffer.rs +++ b/src/buffer.rs @@ -2962,6 +2962,115 @@ mod tests { ] } + /// Provenance of the `Edit` a [`GenOp`] produced. + /// + /// The `crdt_op` shape invariant is keyed on THIS, not on the + /// `Edit`'s shape alone. An `Edit` carries no provenance + /// marker, so the classification has to be taken from the + /// operation *before* it is applied — see the call site, where + /// `op` is moved into `apply_capturing`. + #[derive(Clone, Copy, Debug, PartialEq, Eq)] + enum OperationClass { + /// `apply_edit` — `Insert` / `Delete` / `Replace`. + Forward, + /// `undo` / `redo`. + History, + } + + impl OperationClass { + fn of(op: &GenOp) -> Self { + match op { + GenOp::Insert(..) | GenOp::Delete(..) | GenOp::Replace(..) => Self::Forward, + GenOp::Undo | GenOp::Redo => Self::History, + } + } + } + + /// The `crdt_op` shape invariant, over three independent axes. + /// + /// The axes are **provenance** (forward vs. history), the + /// **text delta** (empty vs. real), and whether a **CRDT op** is + /// carried. They are independent, which is the whole point of + /// this lane, so the rule is a full enumeration rather than a + /// default with exceptions: + /// + /// | provenance | text delta | `crdt_op` | verdict | + /// |---|---|---|---| + /// | forward | empty | `None` | **valid** — a syntactic no-op | + /// | forward | empty | `Some` | **invalid** | + /// | forward | real | `Some` | **valid** | + /// | forward | real | `None` | **invalid** | + /// | history | empty | `Some` | **valid** — a version-only edit | + /// | history | empty | `None` | **invalid** | + /// | history | real | `Some` | **valid** | + /// | history | real | `None` | **invalid** | + /// + /// An **empty text delta** — `range.is_empty() && inserted_len + /// == 0` — is a SHAPE, and forward edits reach it routinely: + /// each of the three syntactically empty `EditOp` forms produces + /// exactly this shape. What separates the two empty-delta cases + /// is the op. Forward, `is_no_op_edit` short-circuits before the + /// CRDT path exists, so there is nothing to carry. History + /// diffs two ropes; an identity replace makes them equal, so the + /// op IS the content of the edit and **dropping it loses the + /// version advance**. That is why the history row demands + /// `Some` rather than merely tolerating it. + /// + /// A present op is separately required to carry the buffer's + /// peer id and non-empty wire bytes. + /// + /// Returns `Err(reason)` rather than asserting, so the same + /// predicate serves the proptest (over generated sequences) and + /// a directed injection. The injection is not optional: the + /// `(forward, empty, Some)` row is **unreachable from any + /// generated forward input**, because a forward empty form + /// short-circuits and a forward real-delta form is not empty. + // The enumeration IS the contract. `match_same_arms` would have + // the three `Ok(())` rows collapsed into one alternation, which + // is exactly the conflation this lane exists to remove: it would + // stop the table from showing that `(forward, empty, None)` and + // `(history, empty, Some)` are valid for OPPOSITE reasons, and a + // future reader would have no way to see which quadrant a change + // moved. + #[allow(clippy::match_same_arms)] + fn check_crdt_op_shape( + class: OperationClass, + edit: &Edit, + expected_peer_id: u64, + ) -> Result<(), String> { + if let Some(op) = edit.crdt_op.as_ref() { + if op.peer_id != expected_peer_id { + return Err(format!( + "peer_id must thread from CrdtState: got {}, want {expected_peer_id}", + op.peer_id + )); + } + if op.bytes.is_empty() { + return Err("wire bytes must be non-empty".to_owned()); + } + } + let empty_text_delta = edit.range.is_empty() && edit.inserted_len == 0; + match (class, empty_text_delta, edit.crdt_op.is_some()) { + (OperationClass::Forward, true, false) => Ok(()), + (OperationClass::Forward, true, true) => Err( + "a FORWARD edit with an empty text delta must have crdt_op = None: the \ + three syntactically empty EditOp forms short-circuit at is_no_op_edit" + .to_owned(), + ), + (OperationClass::History, true, true) => Ok(()), + (OperationClass::History, true, false) => Err( + "a HISTORY edit with an empty text delta must have crdt_op = Some: the \ + op is the version advance, and without it the edit carries nothing" + .to_owned(), + ), + (_, false, true) => Ok(()), + (_, false, false) => Err( + "an edit with a real text delta must have crdt_op = Some in CRDT mode" + .to_owned(), + ), + } + } + // T M10.2 Day 3 helper: applies a `GenOp` and returns the // resulting Edit so the proptest can assert per-op shape. // Each op is best-effort: out-of-range positions are clamped @@ -3012,35 +3121,43 @@ mod tests { /// version while leaving the materialized text unchanged, so /// `undo_crdt_mode` derives an EMPTY replacement edit — and /// still attaches the `crdt_op` that `crdt.undo()` produced. - /// That trips the proptest's `crdt_op` shape invariant, "a - /// no-op edit must have `crdt_op = None`". /// - /// **What was verified about the consequences**, so the next - /// reader does not have to redo it: + /// **The ruling** (`docs/crdt-identity-undo-framing.md`, and + /// this is no longer an open question): a visible TEXT delta + /// and a CRDT-VERSION delta are INDEPENDENT dimensions of + /// `Edit`, so the behavior is right and the *invariant* was + /// mis-scoped. It was written for [`is_no_op_edit`], a + /// pre-check on the forward `EditOp` that returns before the + /// CRDT path exists; `undo_crdt_mode` and `redo_crdt_mode` + /// never reach it. The invariant is now keyed on **provenance** + /// — see `check_crdt_op_shape` — and still rejects this shape + /// on the forward path, where it remains unreachable. + /// + /// **What is established, and by what:** /// /// * content stays correct — rope and CRDT projection agree /// before and after (asserted below); - /// * replicas stay converged — both `crdt_op` consumers - /// (`EditorCore::queue_daemon_origin_crdt_op` and the remote-op - /// path) read `edit.crdt_op` unconditionally and do **not** - /// short-circuit on an empty range, so the op is broadcast; - /// * the cursor does not jump — `EditorCore::undo` only clamps - /// to buffer length and never seeks `edit.range.start`. + /// * replicas stay converged — established by + /// `identity_replace_history_op_replays_convergently_on_a_remote_replica`, + /// which seeds a second replica with the forward ops and then + /// replays the history op, asserting the materialized text + /// **and** the version vector. Before that witness existed + /// this comment asserted convergence from call-site + /// inspection alone, which cannot see a lost version advance: + /// dropping the op leaves the text identical; + /// * every consumer of the resulting `Edit` is classified inert + /// or permitted — the census is §4 of the framing, and + /// `identity_replace_history_op_leaves_classified_consumers_unchanged` + /// executes it. /// - /// **The open question** is therefore whether the *invariant* is - /// simply mis-scoped rather than the behavior being wrong. It - /// was written for the FORWARD `apply_edit` short-circuit, which - /// returns before ever producing an op; CRDT-mode undo/redo - /// never reach that path. One artifact is genuinely arbitrary - /// either way: `derive_replacement_edit` reports the empty range - /// at the buffer END rather than at the edit site. - /// - /// Ignored, not deleted: it documents a real, reproducible - /// asymmetry that nothing else on `main` records, and un-ignoring - /// it is the first step of whichever resolution wins. + /// **The empty range's location is ruled, not arbitrary.** + /// `derive_replacement_edit` reports it at the buffer END. No + /// consumer is harmed there, and for the one consumer whose + /// cost depends on it — `TextView::on_edit`, which rebuilds + /// from `line_at_offset(range.start)` — the buffer end is the + /// cheapest possible choice. An earlier version of this comment + /// called it genuinely arbitrary; it is weakly preferable. #[test] - #[ignore = "known pre-existing main behavior; see the doc comment \ - for the verified consequences and the open question"] fn crdt_undo_of_an_identity_replace_reports_a_no_op_edit_carrying_an_op() { let mut buffer = Buffer::new_with_crdt(BufferId::next(), "*identity-undo*", 1).expect("crdt"); @@ -3084,6 +3201,484 @@ mod tests { ); } + /// C1: the fixture above must not be silently re-ignored. + /// + /// A restored `#[ignore]` is invisible to a green suite — the + /// run simply reports one fewer test. This reads the source and + /// asserts the attribute's ABSENCE, which is the only form that + /// fails rather than quietly reporting. + #[test] + fn the_identity_replace_fixture_carries_no_ignore_attribute() { + const SOURCE: &str = include_str!("buffer.rs"); + const FIXTURE: &str = + "fn crdt_undo_of_an_identity_replace_reports_a_no_op_edit_carrying_an_op"; + let at = SOURCE + .find(FIXTURE) + .expect("the fixture is present by name"); + let attrs: Vec<&str> = SOURCE[..at] + .lines() + .rev() + .map(str::trim) + .skip_while(|l| l.is_empty()) + .take_while(|l| l.starts_with("#[")) + .collect(); + assert!( + attrs.iter().any(|a| a.starts_with("#[test]")), + "the fixture should still be a #[test]: {attrs:?}" + ); + assert!( + !attrs.iter().any(|a| a.starts_with("#[ignore")), + "C1: the identity-replace fixture is ignored again — {attrs:?}" + ); + } + + /// C2a: the classifier itself, with nothing between the + /// assertion and it. + /// + /// Two of the three end-to-end paths mask a classifier + /// regression (see C2b), so this row exists to be unmaskable. + #[test] + fn is_no_op_edit_classifies_all_three_syntactically_empty_forms() { + assert!(is_no_op_edit(&EditOp::Insert { pos: 0, bytes: b"" })); + assert!(is_no_op_edit(&EditOp::Delete { + range: Range::new(0, 0) + })); + assert!(is_no_op_edit(&EditOp::Replace { + range: Range::new(0, 0), + bytes: b"", + })); + // …and does not over-classify: each form with any content + // is a real edit. + assert!(!is_no_op_edit(&EditOp::Insert { + pos: 0, + bytes: b"x" + })); + assert!(!is_no_op_edit(&EditOp::Delete { + range: Range::new(0, 1) + })); + assert!(!is_no_op_edit(&EditOp::Replace { + range: Range::new(0, 1), + bytes: b"", + })); + } + + /// C2b: end to end, each syntactically empty form still + /// produces no CRDT op. + /// + /// **This witness is masked for two of the three forms**, which + /// is why C2a exists. `apply_to_crdt_then_normalize_bytes` + /// returns `(None, None)` early for an empty `Insert` and an + /// empty `Delete`, so flipping `is_no_op_edit`'s arm for either + /// leaves `crdt_op == None` and this test still passes. Killing + /// it there needs a compound mutant: flip the arm AND delete + /// that variant's defensive early return. The empty `Replace` + /// has no such return and falls through to the unconditional + /// `Some(crdt_op)`, so there the simple mutant does die here. + #[test] + fn each_syntactically_empty_form_yields_no_crdt_op_end_to_end() { + let mut b = Buffer::new_with_crdt(BufferId::next(), "*empty-forms*", 1).expect("crdt"); + b.apply_edit(EditOp::Insert { + pos: 0, + bytes: b"seed", + }) + .expect("seed"); + + for (label, op) in [ + ("Insert{bytes:[]}", EditOp::Insert { pos: 0, bytes: b"" }), + ( + "Delete{range:empty}", + EditOp::Delete { + range: Range::new(1, 1), + }, + ), + ( + "Replace{range:empty,bytes:[]}", + EditOp::Replace { + range: Range::new(1, 1), + bytes: b"", + }, + ), + ] { + let edit = b.apply_edit(op).expect("empty form applies"); + assert!( + edit.crdt_op.is_none(), + "C2b: {label} must produce no CRDT op" + ); + assert!( + edit.range.is_empty() && edit.inserted_len == 0, + "C2b: {label} must be version-only in shape too" + ); + } + } + + /// C5: the invariant is keyed on PROVENANCE, and covers all + /// four empty-text-delta quadrants. + /// + /// Two of these must be a directed injection rather than a + /// property. `(forward, empty, Some)` is **unreachable from any + /// generated forward input** — an empty form short-circuits + /// before the CRDT path, and a real-delta form is not empty — so + /// the proptest alone cannot tell a narrowed rule from a deleted + /// one. `(history, empty, None)` is equally unreachable, because + /// `undo_crdt_mode` and `redo_crdt_mode` always attach the op; + /// it is asserted so that a future change which stops attaching + /// it fails here rather than silently losing version advances. + #[test] + fn the_shape_invariant_covers_all_four_empty_text_delta_quadrants() { + let with_op = |op: Option>| Edit { + new_rope: crate::rope::Rope::from_bytes(b"hello"), + range: Range::new(5, 5), + inserted_len: 0, + crdt_op: op, + }; + let carrying = || { + Some(Box::new(crate::rope::CrdtOp { + peer_id: 1, + bytes: vec![0xAB], + })) + }; + + // Forward + empty delta + None: a syntactic no-op. Valid. + assert!( + check_crdt_op_shape(OperationClass::Forward, &with_op(None), 1).is_ok(), + "C5: a forward syntactic no-op carries no op, and that is correct" + ); + // Forward + empty delta + Some: the original bug. + assert!( + check_crdt_op_shape(OperationClass::Forward, &with_op(carrying()), 1).is_err(), + "C5: a forward edit with an empty text delta must not carry an op" + ); + // History + empty delta + Some: a version-only edit. Valid. + assert!( + check_crdt_op_shape(OperationClass::History, &with_op(carrying()), 1).is_ok(), + "C5: the same shape from undo/redo is a legitimate version advance" + ); + // History + empty delta + None: the version advance is gone. + assert!( + check_crdt_op_shape(OperationClass::History, &with_op(None), 1).is_err(), + "C5: a history edit with an empty text delta and no op carries nothing at all" + ); + } + + /// The two history operations every history witness must cover. + #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] + enum HistoryCase { + Undo, + Redo, + } + + impl HistoryCase { + const ALL: [HistoryCase; 2] = [HistoryCase::Undo, HistoryCase::Redo]; + + /// The `match` is the growth guard: a new variant fails to + /// compile here rather than going silently unexercised. + fn label(self) -> &'static str { + match self { + Self::Undo => "undo", + Self::Redo => "redo", + } + } + } + + /// C6: assert the executed case set IS `{Undo, Redo}`. + /// + /// Narrowing a parameterized loop from two cases to one + /// ordinarily leaves a passing test — the suite just runs less, + /// which no assertion inside the loop can notice. The expected + /// set is spelled out literally rather than derived from + /// `HistoryCase::ALL`, which would make the check circular. + fn assert_executed_both_history_cases(executed: &[HistoryCase]) { + let mut got = executed.to_vec(); + got.sort(); + got.dedup(); + assert_eq!( + got, + vec![HistoryCase::Undo, HistoryCase::Redo], + "C6: the history witnesses must execute BOTH cases" + ); + } + + /// A buffer holding "hello" whose last edit was an identity + /// replace — the shape whose undo and redo are version-only. + fn seeded_identity_replace_buffer() -> Buffer { + let mut b = + Buffer::new_with_crdt(BufferId::next(), "*identity-history*", 1).expect("crdt"); + b.apply_edit(EditOp::Insert { + pos: 0, + bytes: b"hello", + }) + .expect("seed insert"); + b.apply_edit(EditOp::Replace { + range: Range::new(1, 2), + bytes: b"e", + }) + .expect("identity replace"); + b + } + + /// Run `case`'s history operation, asserting it is version-only. + fn take_history_edit(b: &mut Buffer, case: HistoryCase) -> Edit { + let edit = match case { + HistoryCase::Undo => b.undo().expect("undo"), + HistoryCase::Redo => b.redo().expect("redo"), + }; + assert!( + edit.range.is_empty() && edit.inserted_len == 0, + "{}: expected a version-only edit, got {:?}/{}", + case.label(), + edit.range, + edit.inserted_len + ); + edit + } + + fn assert_converged(a: &Buffer, replica: &crate::crdt::CrdtState, when: &str) { + let doc = a.crdt_state().expect("crdt"); + assert_eq!( + doc.materialize_string(), + replica.materialize_string(), + "text diverged {when}" + ); + // The VERSION is the discriminator, and `version_scalar` is + // documented as unusable for exactly this comparison + // (equal scalars do not imply equal states across + // replicas). `VersionVector`'s `PartialEq` compares logical + // content. + assert_eq!( + doc.version(), + replica.version(), + "version vector diverged {when}" + ); + } + + /// C3: an empty-text history op replays convergently on a + /// REMOTE replica, for both `undo` and `redo`. + /// + /// The existing round-trip proptest deliberately excludes + /// history ops, because replaying one onto a replica that never + /// saw the forward history is ill-posed. Seeding the replica + /// with the forward ops first is what makes this case well + /// posed — and is the shape the wire protocol actually uses. + /// + /// **Text equality alone does not discriminate.** Dropping the + /// history op leaves the replica's text identical, because the + /// op advances the version without changing bytes. The version + /// vector is what catches it. + #[test] + fn identity_replace_history_op_replays_convergently_on_a_remote_replica() { + let mut executed = Vec::new(); + for case in HistoryCase::ALL { + let mut a = Buffer::new_with_crdt(BufferId::next(), "*replay*", 1).expect("crdt"); + let replica = crate::crdt::CrdtState::new(2).expect("replica"); + + for op in [ + EditOp::Insert { + pos: 0, + bytes: b"hello", + }, + EditOp::Replace { + range: Range::new(1, 2), + bytes: b"e", + }, + ] { + let edit = a.apply_edit(op).expect("forward edit"); + let carried = edit.crdt_op.as_ref().expect("a forward edit carries an op"); + replica + .import_updates(&carried.bytes) + .expect("seed the replica"); + } + assert_converged(&a, &replica, "after seeding the forward ops"); + + // Redo needs an undo first — and that undo is itself a + // version-only edit, so it is replayed the same way. + if case == HistoryCase::Redo { + let undone = take_history_edit(&mut a, HistoryCase::Undo); + replica + .import_updates(&undone.crdt_op.as_ref().expect("op").bytes) + .expect("replay the preparatory undo"); + assert_converged(&a, &replica, "after the preparatory undo"); + } + + let history = take_history_edit(&mut a, case); + let carried = history + .crdt_op + .as_ref() + .expect("the history op carries a version advance"); + replica + .import_updates(&carried.bytes) + .expect("replay the history op"); + assert_converged(&a, &replica, case.label()); + + // Corroboration: a causally dependent op still lands. + let follow = a + .apply_edit(EditOp::Insert { + pos: a.len(), + bytes: b"!", + }) + .expect("dependent edit"); + replica + .import_updates(&follow.crdt_op.as_ref().expect("op").bytes) + .expect("replay the dependent op"); + assert_converged(&a, &replica, "after a causally dependent op"); + + executed.push(case); + } + assert_executed_both_history_cases(&executed); + } + + /// C4a: the history edit is BROADCAST at all. + /// + /// This is what makes C4b non-vacuous. C4b asserts that the + /// classified consumers are unchanged, and "unchanged" is also + /// what a missing broadcast produces — so the census's whole + /// broadcast branch rests on this count. + #[test] + fn identity_replace_history_op_is_broadcast_to_attached_views() { + let mut executed = Vec::new(); + for case in HistoryCase::ALL { + let mut b = seeded_identity_replace_buffer(); + let events = std::sync::Arc::new(Mutex::new(Vec::new())); + b.attach_view(Box::new(RecorderView { + events: std::sync::Arc::clone(&events), + })); + if case == HistoryCase::Redo { + take_history_edit(&mut b, HistoryCase::Undo); + } + events.lock().unwrap().clear(); + + take_history_edit(&mut b, case); + + let broadcasts = events + .lock() + .unwrap() + .iter() + .filter(|e| matches!(e, RecorderEvent::OnEdit { .. })) + .count(); + assert_eq!( + broadcasts, + 1, + "C4a: {} must broadcast exactly one on_edit", + case.label() + ); + executed.push(case); + } + assert_executed_both_history_cases(&executed); + } + + /// C4b: §4's classification, executed. + /// + /// The three production views that override `on_edit` are + /// attached to one buffer, the identity-replace history op runs, + /// and each consumer's classified outcome is asserted: the fold + /// store and the span vector unchanged (INERT), and `ParseView` + /// left with an identical parse and a queue that drains + /// (PERMITTED — one degenerate `InputEdit`, describing no + /// change). + #[test] + fn identity_replace_history_op_leaves_classified_consumers_unchanged() { + use crate::overlay::{ + BufferStyleSpan, BufferStyleSpanTranslator, SharedBufferStyleSpans, + }; + use pmacs_protocol::ByteRange; + + let registry = crate::syntax::SyntaxRegistry::new(); + let Some(language) = registry.language("rust") else { + panic!("the rust grammar must load for C4b"); + }; + + let mut executed = Vec::new(); + for case in HistoryCase::ALL { + let mut b = seeded_identity_replace_buffer(); + + let folds = crate::fold::FoldRegistry::default(); + let store = folds.store_or_attach(&mut b); + assert!( + store.lock().unwrap().insert(ByteRange { start: 1, end: 4 }), + "a fold to observe" + ); + + let spans: SharedBufferStyleSpans = + std::sync::Arc::new(Mutex::new(vec![BufferStyleSpan { + start: 1, + end: 4, + style: crate::cell::Style::default(), + }])); + b.attach_view(Box::new(BufferStyleSpanTranslator::new( + std::sync::Arc::clone(&spans), + ))); + + let parse_view = crate::syntax::ParseView::new(&b, language.clone(), "rust".into()); + let handle = parse_view.handle(); + b.attach_view(Box::new(parse_view)); + + if case == HistoryCase::Redo { + take_history_edit(&mut b, HistoryCase::Undo); + } + + // Baselines, taken after any preparatory op so the + // comparison is against the state the op under test + // actually starts from. + let folds_before = store.lock().unwrap().folds(); + let spans_before = spans.lock().unwrap().clone(); + let source_before = handle.source_snapshot(); + let baseline = std::sync::Arc::new( + crate::syntax::run_parse(handle.make_request()).expect("baseline parse"), + ); + handle.install(std::sync::Arc::clone(&baseline)); + let tree_before = baseline.root_tree().root_node().to_sexp(); + assert_eq!( + handle.pending_edit_count(), + 0, + "the baseline parse drains the queue" + ); + + take_history_edit(&mut b, case); + + assert_eq!( + store.lock().unwrap().folds(), + folds_before, + "C4b: FoldStoreTranslator is INERT for {}", + case.label() + ); + assert_eq!( + *spans.lock().unwrap(), + spans_before, + "C4b: BufferStyleSpanTranslator is INERT for {}", + case.label() + ); + assert_eq!( + handle.source_snapshot(), + source_before, + "C4b: ParseView's source mirror is unchanged for {}", + case.label() + ); + // The permitted effect, bounded: exactly one degenerate + // InputEdit is queued, and the next request drains it. + assert_eq!( + handle.pending_edit_count(), + 1, + "C4b: one degenerate InputEdit for {}", + case.label() + ); + let after = crate::syntax::run_parse(handle.make_request()).expect("parse again"); + assert_eq!( + handle.pending_edit_count(), + 0, + "C4b: the queue drains for {}", + case.label() + ); + assert_eq!( + after.root_tree().root_node().to_sexp(), + tree_before, + "C4b: the parse is identical for {}", + case.label() + ); + + executed.push(case); + } + assert_executed_both_history_cases(&executed); + } + proptest! { // Smaller proptest case count than the default (64) to keep // CI overhead modest; the per-op invariant check is the @@ -3098,6 +3693,10 @@ mod tests { .expect("crdt construction"); for op in ops { let op_repr = format!("{op:?}"); + // C5: classify BEFORE the move. This is the only + // point at which forward and history are + // distinguishable; the resulting `Edit` is not. + let class = OperationClass::of(&op); let edit = apply_capturing(&mut b, op); // Per-op invariant check: catches drift the moment // it happens, with the failing op visible in the @@ -3109,37 +3708,13 @@ mod tests { "invariant violated after op {}: rope={:?} crdt={:?}", op_repr, rope, crdt ); - // Day 3: crdt_op shape invariant. - // - real edits in CRDT mode populate crdt_op - // - no-op short-circuits leave crdt_op = None - // - history-stack-empty errors return None Edit - if let Some(edit) = edit { - let is_no_op_edit_result = - edit.range.is_empty() && edit.inserted_len == 0; - if is_no_op_edit_result { - prop_assert!( - edit.crdt_op.is_none(), - "no-op edit must have crdt_op = None ({})", - op_repr - ); - } else { - prop_assert!( - edit.crdt_op.is_some(), - "non-no-op CRDT-mode edit must have crdt_op = Some ({})", - op_repr - ); - let crdt_op = edit.crdt_op.as_ref().unwrap(); - prop_assert_eq!( - crdt_op.peer_id, 1, - "peer_id must thread from CrdtState ({})", - op_repr - ); - prop_assert!( - !crdt_op.bytes.is_empty(), - "wire bytes must be non-empty ({})", - op_repr - ); - } + // Day 3: crdt_op shape invariant, now keyed on + // provenance rather than on the Edit's shape alone. + // A history-stack-empty error returns no Edit. + if let Some(edit) = edit + && let Err(why) = check_crdt_op_shape(class, &edit, 1) + { + prop_assert!(false, "{} ({})", why, op_repr); } } } diff --git a/src/overlay.rs b/src/overlay.rs index e02dd9e..91b4b12 100644 --- a/src/overlay.rs +++ b/src/overlay.rs @@ -753,6 +753,52 @@ mod tests { virt.render(&buf, viewport(1, 5), &mut grid); } + /// C4c: the empty/empty guard's own contract, pinned where it + /// can actually fire. + /// + /// **The history edit this guard was written for cannot test it.** + /// That edit sits at the buffer END, and with + /// `old_start == old_end == len` the loop below emits a left + /// fragment `[s, min(e, len)) == [s, e)` for every span within the + /// buffer and no right fragment — the vector is unchanged with or + /// without the guard, so deleting the guard is a surviving mutant + /// there. An INTERIOR empty edit is where the fragmenting the + /// guard prevents is reachable: a span straddling the position + /// splits into two adjacent fragments covering the same bytes. + /// + /// So the assertion is on the span VECTOR, not on coverage. + #[test] + fn an_interior_empty_edit_does_not_fragment_a_straddled_span() { + let buf = Buffer::from_bytes(BufferId::next(), "t", b"abcdef"); + let store: SharedBufferStyleSpans = Arc::new(Mutex::new(vec![BufferStyleSpan { + start: 1, + end: 5, + style: red(), + }])); + let before = store.lock().unwrap().clone(); + let mut translator = BufferStyleSpanTranslator::new(Arc::clone(&store)); + + // A synthetic 0→0 edit strictly inside the span. Built by hand: + // no forward EditOp produces this shape, and the history edit + // that does produce it lands at the buffer end. + let interior = crate::rope::Edit { + new_rope: buf.snapshot_rope(), + range: crate::rope::Range::new(3, 3), + inserted_len: 0, + crdt_op: None, + }; + translator + .on_edit(&buf, &interior) + .expect("the translator accepts the edit"); + + assert_eq!( + *store.lock().unwrap(), + before, + "C4c: an interior no-op edit must leave the span vector \ + byte-identical, not split it into adjacent fragments" + ); + } + fn red() -> Style { Style { fg: crate::cell::Color::Indexed(1), diff --git a/src/rope.rs b/src/rope.rs index 3081158..1b1b40e 100644 --- a/src/rope.rs +++ b/src/rope.rs @@ -301,6 +301,27 @@ impl<'a> Iterator for Chunks<'a> { /// A pure insert has `range.start == range.end` and `inserted_len > 0`. /// A pure delete has `range.start < range.end` and `inserted_len == 0`. /// A replace has both nonzero. +/// An **empty text delta** has `range.start == range.end` and +/// `inserted_len == 0` — no bytes changed at all. +/// +/// That last shape is produced on BOTH paths, and `crdt_op` is what +/// tells them apart: +/// +/// * **forward** `apply_edit` reaches it whenever the `EditOp` is one of +/// the three syntactically empty forms (an empty insert, an +/// empty-range delete, an empty-range/empty-bytes replace). Those +/// short-circuit before the CRDT path exists, so `crdt_op` is `None` +/// — nothing happened; +/// * **CRDT-mode `undo`/`redo`** reach it when the operation being +/// inverted was itself a textual no-op (replacing bytes with +/// identical bytes). Here `crdt_op` is `Some`, and **must be**: a CRDT +/// VERSION delta is a separate dimension from a TEXT delta, and the +/// op is the whole content of such an edit. +/// +/// The history case's `range` sits at the buffer end, which is where +/// `derive_replacement_edit` reports a no-difference diff; see +/// `docs/crdt-identity-undo-framing.md` for the consumer census that +/// ruled that location harmless. #[derive(Clone, Debug)] pub struct Edit { /// The rope after the edit. `Send + Sync`; safe to hand to a worker. @@ -312,9 +333,17 @@ pub struct Edit { /// T M10.2 Day 3: optional CRDT-op metadata. /// /// `Some` when this Edit was produced by a CRDT-backed Buffer's - /// edit path (`apply_edit` / `undo` / `redo`); `None` otherwise — both - /// in v0.1 mode (no CRDT) and for no-op edits in CRDT mode (an - /// empty insert at an empty range produces no CRDT op). + /// edit path (`apply_edit` / `undo` / `redo`); `None` otherwise — + /// in v0.1 mode (no CRDT), and in CRDT mode for the three + /// syntactically empty `EditOp` forms, which `is_no_op_edit` + /// short-circuits before the CRDT path runs. + /// + /// An empty text delta is therefore NOT by itself a `None` signal: + /// forward, it means the edit short-circuited and `crdt_op` is + /// `None`; from `undo`/`redo` it means an identity operation was + /// inverted, and `crdt_op` is `Some` — and must be, or the version + /// advance the replicas need is lost. See the shape list on + /// [`Edit`] above. /// /// `Box` indirection: keeps Edit's None-case cost to 8 bytes /// (Box has a niche-optimized None) rather than the ~32 bytes diff --git a/src/view.rs b/src/view.rs index 06bc7dc..2e7c658 100644 --- a/src/view.rs +++ b/src/view.rs @@ -597,4 +597,119 @@ mod tests { assert_eq!(vp.row_offset_of(0, 3), None, "hidden lines have no row"); assert_eq!(vp.row_offset_of(0, 5), Some(2), "rows below shift up"); } + + /// C9: §4 of `docs/crdt-identity-undo-framing.md` enumerates every + /// in-tree `on_edit` override so that the consumers of a + /// version-only history `Edit` are a closed set. This asserts the + /// set is still what the census measured. + /// + /// **It asserts PAIRS, not a file set and a count.** Replacing + /// `ParseView`'s override with an unclassified type in the same + /// file leaves both the file set and the total unchanged, and only + /// the pair set catches it. + /// + /// **Its reach is in-tree, and that is a real limit.** [`View`] and + /// `Buffer::attach_view` are both public, so a downstream crate may + /// implement `on_edit` and attach it; no in-tree measurement can + /// enumerate that. What speaks to those implementors is the + /// documented contract on `Edit` itself. + /// + /// It also guards only the census's CLOSURE condition — that the + /// override set is unchanged — not the classifications inside it. + /// Those are executed by + /// `identity_replace_history_op_leaves_classified_consumers_unchanged`. + #[test] + fn every_in_tree_on_edit_override_is_one_the_census_classified() { + /// `(file, impl target)`, as measured by the census. + const CLASSIFIED: [(&str, &str); 4] = [ + ("fold.rs", "FoldStoreTranslator"), + ("overlay.rs", "BufferStyleSpanTranslator"), + ("syntax.rs", "ParseView"), + ("text_view.rs", "TextView"), + ]; + + fn rs_files(dir: &std::path::Path, out: &mut Vec) { + for entry in std::fs::read_dir(dir).expect("src is readable") { + let path = entry.expect("dir entry").path(); + if path.is_dir() { + rs_files(&path, out); + } else if path.extension().is_some_and(|e| e == "rs") { + out.push(path); + } + } + } + + let mut files = Vec::new(); + rs_files( + &std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"), + &mut files, + ); + files.sort(); + + let mut found: Vec<(String, String)> = Vec::new(); + for path in &files { + let name = path + .file_name() + .expect("file name") + .to_string_lossy() + .into_owned(); + let text = std::fs::read_to_string(path).expect("source is readable"); + let lines: Vec<&str> = text.lines().collect(); + // The test module boundary: the first `#[cfg(test)]` that + // introduces a `mod`. Overrides below it are test fixtures + // and are out of scope, as the census says. + let boundary = lines.iter().enumerate().find_map(|(i, l)| { + (l.trim() == "#[cfg(test)]" + && lines + .get(i + 1) + .is_some_and(|n| n.trim_start().starts_with("mod "))) + .then_some(i) + }); + for (i, line) in lines.iter().enumerate() { + if boundary.is_some_and(|b| i > b) { + break; + } + if !line.contains("fn on_edit") { + continue; + } + // Walk back to the enclosing `impl … for `. A hit + // on a trait declaration first means this is the + // trait's own default, which is not an override. + for j in (0..=i).rev() { + let l = lines[j].trim_start(); + if let Some(rest) = l.strip_prefix("impl") + && let Some(after) = rest.split(" for ").nth(1) + { + let target = after + .split_whitespace() + .next() + .unwrap_or("") + .trim_end_matches('{') + .rsplit("::") + .next() + .unwrap_or("") + .to_owned(); + found.push((name.clone(), target)); + break; + } + if l.starts_with("trait ") || l.starts_with("pub trait ") { + break; + } + } + } + } + found.sort(); + + let expected: Vec<(String, String)> = CLASSIFIED + .iter() + .map(|(f, t)| ((*f).to_owned(), (*t).to_owned())) + .collect(); + assert_eq!( + found, expected, + "C9: the in-tree `on_edit` override set no longer matches the \ + census in docs/crdt-identity-undo-framing.md §4. Every \ + override is a consumer of the version-only history `Edit` \ + and needs classifying there." + ); + } }