From 94036774a8ddda4120b5b9b40f2430c9b67aa50a Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Fri, 31 Jul 2026 16:43:02 -0400 Subject: [PATCH] fix(welcome): notify the core after writing scratch; unstale the ledger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1 on #205, two findings, both accepted. The greeting was written straight into the registry without calling `notify_buffer_edit`. The window's `TextView` had been indexed while `*scratch*` was empty, and newlines are zero-width to a painter working from a stale line index — so the first TUI frame collapsed the whole three-line greeting onto row 0. Every buffer-text assertion passed because the buffer content was correct; only the rendering was wrong. The edit is now captured, the registry borrow released, and the core notified. The pin that would have caught it paints a real frame and asserts the second line occupies its own row AND that row 0 does not contain it — both directions, because a one-direction check passes when everything collapses upward. Bitten by dropping the notify call: row 1 comes back empty with row 0 holding the lot, and it is the only pin that fails. Second: the project docs still described the arc as it was two PRs ago. `COHERENCE.md` §20 called 1b-2 in flight and the welcome buffer unstarted; its arc list said 1b-3 remained; and the ledger's journey lane header still read "1b-2 PR OPEN" while the 1b-3 block carried a mangled "Framing only; no code" line left by an earlier edit. All now describe the PR-head state per §25. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T --- COHERENCE.md | 22 ++++++------ docs/active-work.md | 11 +++--- src/editor.rs | 29 ++++++++++------ tests/journey_acceptance.rs | 68 +++++++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 25 deletions(-) diff --git a/COHERENCE.md b/COHERENCE.md index 5e723e4..43dd0ec 100644 --- a/COHERENCE.md +++ b/COHERENCE.md @@ -1575,19 +1575,20 @@ Establish the end-to-end workflow; treat regressions as release blockers. **State: runs to step 5; thin from step 6 (§2). Mostly wiring, and unusually cheap:** directory-argument handling (**done**: Journey Stage 1a); a find-file surface (**done**: #162 open-by-path, #165 -browsing); surfacing the LSP spawn failure with guidance (**in flight**: -Journey Stage 1b-2, §1.2); a compile keybinding + `cargo build`/`test` -default (**done**: Journey Stage 1b-1, #203, from the existing +browsing); surfacing the LSP spawn failure with guidance (**done**: Journey Stage +1b-2, #204, §1.2); a compile keybinding + `cargo build`/`test` default +(**done**: Journey Stage 1b-1, #203, from the existing `ProjectKind::Rust` — **not** `Cargo`, see §24); a terminal keybinding -(**done**: `C-c t`, #173); a welcome buffer. The journey acceptance +(**done**: `C-c t`, #173); a welcome buffer (**in flight**: Journey +Stage 1b-3). The journey acceptance suite (§19) is the ratchet that keeps it fixed — it **exists now** (`tests/journey_acceptance.rs`, Stage 1a), seeded with steps 2, 3 and 5, and carrying step 9 since #203. -Journey Stage 1b is the named remainder, and it splits: **1b-1 — the -compile binding + project-kind defaults — landed as #203**; **1b-2** -(LSP spawn guidance, step 6) is in flight; **1b-3**, the welcome buffer -(step 4), remains. +Journey Stage 1b is the named remainder, and it splits: **1b-1** (the +compile binding + project-kind defaults) landed as #203 and **1b-2** +(LSP spawn guidance, step 6) as #204; **1b-3**, the welcome buffer +(step 4), is in flight and completes the split. ### Priority 2: Make workspace and location explicit @@ -1660,8 +1661,9 @@ implementation — this list is direction, not commitment): #165's dired buffer rather than growing a second directory surface. **Stage 1b-1 — landed (#203)**: the compile binding and project-kind defaults, with the prompt capturing its directory rather than - re-resolving it at accept time. **Stage 1b-2 — in flight**: - LSP-failure surfacing. **Stage 1b-3 — remaining**: welcome buffer. + re-resolving it at accept time. **Stage 1b-2 — landed (#204)**: + LSP-failure surfacing. **Stage 1b-3 — in flight**: the welcome + buffer and `M-x help`. With it the 1b split is complete. 2. **Discovery surface** (P4): the describe/list/where-is command family, M-x rich rows, help unification, help prefix. 3. **Transient keymap layer** (§6): the overlay scope + lifetime diff --git a/docs/active-work.md b/docs/active-work.md index 315422f..1871bc6 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -255,11 +255,11 @@ If it does not, stop and repair the remote/fetch configuration. never been enforced. Any CI job that compiles the `crdt` targets has to fix them first or it will be red on arrival. -## Journey lane (P1) — 1a and 1b-1 MERGED; 1b-2 PR OPEN; 1b-3 REMAINS +## Journey lane (P1) — 1a, 1b-1, 1b-2 MERGED; 1b-3 PR OPEN -**Rewritten, not removed, at #203's merge.** Rule 4 removes a lane when -its ARC is done; the journey arc is not — 1b-2 is in flight and 1b-3 is -unframed. Stage 1a (#182/#183) and Stage 1b-1 (#203) are on `main` and +**Rewritten, not removed.** Rule 4 removes a lane when its ARC is done; +the journey arc is not — 1b-3 is the last stage of the 1b split and is +still open. Stage 1a (#182/#183) and Stage 1b-1 (#203) are on `main` and their durable facts are in `docs/agent-handoff.md` §1, which is rule 4's precondition satisfied rather than deferred. @@ -326,7 +326,8 @@ re-conflict on every merge. ## Journey Stage 1b-3 (P1) — IMPLEMENTED, PR OPEN - **Branch `journey-stage1b3-welcome`**, worktree `../pmacs-journey-1b3`, - based on `githubsucks/main` @ `1f290d5`. **Framing only; no code, no + based on `githubsucks/main` @ `1f290d5`, **integrated with `main` @ + `5376af1`** (#204). **Implemented; PR #205 open.** `docs/journey-stage1b3-welcome-framing.md` revision 4, three review rounds closed (round 1: four findings; round 2: two acceptance holes plus a doc correction; round 3: a visibility mismatch and an diff --git a/src/editor.rs b/src/editor.rs index 12e755b..b089973 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -940,20 +940,29 @@ impl EditorState { // gets no greeting, not a failed launch. Err(_) => return, }; + // Capture the edit and RELEASE the registry borrow before + // notifying: `notify_buffer_edit` borrows the registry itself. let registry = self.core.borrow().registry.clone(); - let mut reg = registry.borrow_mut(); - let Ok(buf) = reg.get_mut(buffer_id) else { - return; - }; - if buf - .apply_edit(crate::buffer::EditOp::Insert { + let edit = { + let mut reg = registry.borrow_mut(); + let Ok(buf) = reg.get_mut(buffer_id) else { + return; + }; + let Ok(edit) = buf.apply_edit(crate::buffer::EditOp::Insert { pos: 0, bytes: text.as_bytes(), - }) - .is_ok() - { + }) else { + return; + }; buf.mark_clean(); - } + edit + }; + // Without this the greeting renders as ONE row on the first + // frame: the window's `TextView` indexed `*scratch*` while it + // was empty, and newlines are zero-width to a painter working + // from a stale line index. Every other direct-registry writer + // notifies for the same reason. + self.core.borrow_mut().notify_buffer_edit(buffer_id, &edit); } /// Restore the session saved under this desktop's key, if armed diff --git a/tests/journey_acceptance.rs b/tests/journey_acceptance.rs index 4dd732d..db88942 100644 --- a/tests/journey_acceptance.rs +++ b/tests/journey_acceptance.rs @@ -2124,3 +2124,71 @@ fn journey_step6_a_missing_language_server_is_reported_not_swallowed() { is what made highlighting able to mask this" ); } + +/// **N** — the greeting renders as separate rows on the **first frame**. +/// +/// Writing to the registry without `notify_buffer_edit` leaves the +/// window's `TextView` indexed against the empty `*scratch*` it was +/// built for. Newlines are zero-width to a painter working from a stale +/// line index, so the whole three-line greeting collapses onto row 0 — +/// visible to a user, invisible to every buffer-text assertion above. +/// +/// Falsified by dropping the `notify_buffer_edit` call in +/// `finalize_local_launch`. +#[test] +fn journey_step4_the_welcome_paints_as_multiple_rows_on_the_first_frame() { + use pmacs::cell::{Cell, CellGrid, CellSize, Glyph}; + + let s = start_local(); + let (rows, cols) = (12u32, 100u32); + let mut cells = vec![Cell::default(); (rows * cols) as usize]; + let mut grid = CellGrid { + cells: &mut cells, + stride: cols, + size: CellSize::new(rows, cols), + }; + let _ = pmacs::editor::paint_frame( + &s, + FrontendId::LOCAL, + &std::collections::HashMap::new(), + &mut grid, + CellSize::new(rows, cols), + ); + + let row_text = |row: u32| -> String { + (0..cols) + .map( + |column| match &cells[(row * cols + column) as usize].glyph { + Glyph::Char(ch) => *ch, + Glyph::Cluster(bytes) => std::str::from_utf8(bytes) + .ok() + .and_then(|t| t.chars().next()) + .unwrap_or(' '), + Glyph::Continuation => ' ', + }, + ) + .collect::() + .trim_end() + .to_owned() + }; + + assert!( + row_text(0).contains("Welcome to pmacs"), + "row 0 is the greeting's first line; got {:?}", + row_text(0) + ); + // The discriminating half: with a stale TextView these land on row 0 + // too, and row 1 is blank. + assert!( + row_text(1).contains("C-x C-f"), + "the second line must occupy its own row, not collapse into the \ + first; row 1 = {:?}, row 0 = {:?}", + row_text(1), + row_text(0) + ); + assert!( + !row_text(0).contains("C-x C-f"), + "and must not have been folded into row 0; got {:?}", + row_text(0) + ); +}