fix(compile): PR #113 round 6 — idempotent split-complete attachment, no-op edit guard, handle disposal

Finding-by-finding (framing revision 12; bites via scripts/bite
against a49adc2):

1. Render-view attachment is idempotent and split-complete. Overlays
   expose overlay_identity (the span store's allocation address);
   Window::ensure_overlay attaches a store-backed render view AT
   MOST once per window — pre-fix every switch into the buffer
   blindly pushed another copy onto EVERY matching window, so
   passive panes accumulated duplicates, each cloning all spans and
   rescanning the buffer per frame. A same-buffer split copies
   clonable overlays to the new pane via clone_for_split (splits
   fire no switch hook and started with an empty overlay list — the
   new compilation pane rendered unstyled). Bites: the acceptance
   test asserts both panes styled with exactly one attachment
   IMMEDIATELY post-split (before any switch could heal the pane
   through the attach-to-all path — the first draft asserted only
   after bouncing and was vacuous against the split fix), then
   re-asserts after three bounce cycles; fails against pre-fix
   editor_core.rs (split half) and pre-fix mod.rs (accumulation
   half) independently. Units pin ensure-once and split-copy/no-copy.
2. The translator ignores pure no-op edits (buffers deliberately
   broadcast empty inserts/deletes for callers that count calls):
   pre-fix each interior no-op split the containing span into two
   adjacent fragments — unbounded list growth for repeated no-ops at
   distinct positions, and a no-op at a UTF-8 continuation byte
   minted a mid-codepoint span boundary. Units now cover genuine
   EditOp::Insert (the round-5 "insertion" unit only replaced) and
   no-ops at five interior positions including the continuation
   byte; the Lua twin (r6f2) bites via the overlay.rs swap — as a
   compile failure, since that file also carries the round-6
   identity machinery (weaker evidence, per the bite script's
   caveat; the in-crate unit pins the behavior directly).
3. StyleOverlayHandleLua retains the buffer and translator ViewId
   and exposes idempotent dispose(): detaches the buffer-attached
   translator (later edits stop paying for it) and removes every
   window render view over the store. Documented lifetime contract:
   one handle per buffer incarnation (the compile/REPL discipline)
   needs no disposal — the buffer's death frees it; repeated
   creation on a long-lived buffer must dispose retired handles.
   Bite: r6f3 (translate → dispose → edit must NOT move the span,
   render views gone, double-dispose safe) fails against pre-fix
   mod.rs.

Gates: fmt; clippy workspace all-targets; lib 1534; crdt lib 1708;
compile acceptance 63; crdt acceptance 3; m4 101; m6.4 15; m6.5 11;
m6.8 8; GPU 59; workspace sweep 2523/0 (one m8-class flake, clean on
rerun); git diff --check.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VoiEyuPjoBhvwACf8HAnLB
This commit is contained in:
Levi Neuwirth 2026-07-14 10:17:56 +01:00
parent a49adc2589
commit fe04aa481b
8 changed files with 448 additions and 13 deletions

View File

@ -1,7 +1,33 @@
# Compile-mode — framing (Arc 5 stage 1, terminal)
**Revision 11 — 2026-07-13. Status: implemented on branch
`compile-mode` (PR #113); revisions 711 fold in PR rounds 15.**
**Revision 12 — 2026-07-14. Status: implemented on branch
`compile-mode` (PR #113); revisions 712 fold in PR rounds 16.**
Revision 12 (PR #113 round 6, findings 13): render-view attachment
is idempotent and split-complete. Overlays expose an
`overlay_identity` (the span store's allocation address);
`Window::ensure_overlay` attaches a store-backed render view AT MOST
once per window — pre-fix every switch into the buffer blindly
pushed another copy onto EVERY matching window, so passive panes
accumulated duplicates, each cloning all spans and rescanning the
buffer per frame. A same-buffer split copies clonable overlays to
the new pane via `clone_for_split` (splits fire no switch hook and
started with an empty overlay list — the new compilation pane
rendered unstyled). The translator ignores pure no-op edits
(buffers deliberately broadcast them): pre-fix each interior no-op
split the containing span into adjacent fragments — unbounded list
growth, and a no-op at a UTF-8 continuation byte minted a
mid-codepoint span boundary. And the overlay handle has a teardown
path: `handle:dispose()` idempotently detaches the buffer-attached
translator and removes every window render view over its store —
the documented lifetime contract is one handle per buffer
incarnation (compile/REPL need no disposal; repeated creation on a
long-lived buffer must dispose retired handles or every edit keeps
paying for abandoned translators). Bites: split+bounce per-cell
acceptance (immediate post-split assert, before any switch could
heal the pane), no-op fragmentation Lua twin, dispose Lua twin;
units for genuine insertion, no-op ignore, split copy, and
ensure-once.
Revision 11 (PR #113 round 5, findings 13): style-span coordinate
translation belongs to the BUFFER, not to views — a new
@ -240,8 +266,12 @@ Everything below was verified by reading the code, not the roadmap.
exactly once per edit — Revision 11) plus render-only window
overlays on windows *currently showing* the buffer; buffer
switches clear window overlays; `attach_style_overlay(buf,
handle)` re-attaches the render view. The handle has `add`,
`clear`, `clear_before`, `spans`.
handle)` re-attaches the render view, idempotently per window via
the store identity, and same-buffer splits copy the render view to
the new pane (Revision 12). The handle has `add`, `clear`,
`clear_before`, `spans`, and idempotent `dispose` (teardown of the
translator + every window render view; one handle per buffer
incarnation needs no disposal).
- **Buffer-switch hooks**: `buffer.after-switch` exists and fires on
the ordinary switch paths (recentf subscribes,
`builtin/runtime/recentf.lua:54`). **`pmacs.editor.jump_back` does

View File

@ -5778,6 +5778,68 @@ mod tests {
);
}
/// PR #113 round-6 finding 1: a same-buffer split copies
/// store-backed render overlays to the new pane (splits fire no
/// switch hook and started from an empty overlay list), and
/// per-window attachment is idempotent via the store identity.
#[test]
fn same_buffer_split_copies_style_overlays_and_attach_is_idempotent() {
use crate::overlay::{BufferStyleOverlay, SharedBufferStyleSpans};
use crate::window::Orientation;
use std::sync::{Arc, Mutex};
let s = fresh_with(b"hello\n");
let store: SharedBufferStyleSpans = Arc::new(Mutex::new(Vec::new()));
{
let mut core = s.core.borrow_mut();
let win = core.active_window_mut();
win.ensure_overlay(Box::new(BufferStyleOverlay::new(Arc::clone(&store))));
// Second ensure over the SAME store: no duplicate.
win.ensure_overlay(Box::new(BufferStyleOverlay::new(Arc::clone(&store))));
assert_eq!(
win.overlay_kinds()
.iter()
.filter(|k| **k == "buffer_style_overlay")
.count(),
1,
"ensure_overlay must be idempotent per store"
);
}
// Same-buffer split: the new pane carries a copy.
let new_id = s
.core
.borrow_mut()
.split_active(Orientation::Horizontal, true);
{
let core = s.core.borrow();
let win = core.windows.get(&new_id).expect("split window");
assert_eq!(
win.overlay_kinds()
.iter()
.filter(|k| **k == "buffer_style_overlay")
.count(),
1,
"a same-buffer split must copy the render overlay"
);
}
// Fresh-buffer split: no copy (different buffer, different
// styling).
let scratch_id = s
.core
.borrow_mut()
.split_active(Orientation::Horizontal, false);
let core = s.core.borrow();
let win = core.windows.get(&scratch_id).expect("scratch window");
assert_eq!(
win.overlay_kinds()
.iter()
.filter(|k| **k == "buffer_style_overlay")
.count(),
0,
"a fresh-buffer split carries nothing"
);
}
// ---- T M2.12: mouse input ----------------------------------------------
fn mouse(kind: crossterm::event::MouseEventKind, row: u16, col: u16) -> MouseEvent {

View File

@ -2109,9 +2109,21 @@ impl EditorCore {
(new_id, TextView::new(buf))
};
let new_id = WindowId::next();
let new_window = Window::new(new_id, buffer_id, text_view);
self.windows.insert(new_id, new_window);
let mut new_window = Window::new(new_id, buffer_id, text_view);
let active = self.active_window_id();
// A same-buffer split starts from an empty overlay list and
// fires no switch hook, so store-backed render overlays
// (ANSI styling on a compile buffer) would silently vanish
// from the new pane (PR #113 round-6 finding 1). Views that
// carry across splits say so via `clone_for_split`.
if same_buffer && let Some(src) = self.windows.get(&active) {
for overlay in &src.overlays {
if let Some(copy) = overlay.clone_for_split() {
new_window.overlays.push(copy);
}
}
}
self.windows.insert(new_id, new_window);
self.active_layout_mut()
.split_window(active, orientation, new_id);
new_id

View File

@ -1743,9 +1743,22 @@ pub struct InterceptHandleLua {
#[derive(Clone)]
/// Lua handle for a shared buffer-byte style overlay.
///
/// Lifetime contract (PR #113 round-6 finding 3): the buffer-attached
/// translator lives until the buffer dies OR `dispose()` is called.
/// One handle per buffer incarnation (the compile-mode and REPL
/// discipline) needs no disposal — the buffer's death frees it;
/// repeated `add_style_overlay` calls on a LONG-LIVED buffer must
/// `dispose()` retired handles, or every edit keeps paying for every
/// abandoned translator.
pub struct StyleOverlayHandleLua {
/// Shared style spans rendered by every attached overlay view.
spans: crate::overlay::SharedBufferStyleSpans,
/// Buffer the translator was attached to.
buffer: BufferId,
/// The buffer-attached translator's view id — retained so
/// `dispose()` can detach it.
translator: crate::buffer::ViewId,
}
impl FromLua for StyleOverlayHandleLua {
@ -1821,6 +1834,29 @@ impl UserData for StyleOverlayHandleLua {
}
Ok(out)
});
// Idempotent teardown (round-6 finding 3): detaches the
// buffer-attached translator (so later edits stop paying for
// it) and removes every window render view over this store.
// Without this, a retired handle's translator lived until
// the buffer died — permanent per-edit cost growth for
// repeated creation on a long-lived buffer. Safe to call
// twice; safe after the buffer is gone.
methods.add_method("dispose", |lua, this, ()| {
let id = crate::overlay::style_store_identity(&this.spans);
if let Some(core) = lua.app_data_ref::<SharedCore>() {
let mut core = core.borrow_mut();
for win in core.windows.values_mut() {
win.overlays.retain(|v| v.overlay_identity() != Some(id));
}
let registry = core.registry.clone();
let mut r = registry.borrow_mut();
if let Ok(buf) = r.get_mut(this.buffer) {
buf.detach_view(this.translator);
}
}
Ok(())
});
}
}
@ -2922,9 +2958,6 @@ fn install_buffer_module(lua: &Lua, registry: &SharedRegistry) -> mlua::Result<T
lua.create_function(
move |lua, id: BufferIdLua| -> mlua::Result<StyleOverlayHandleLua> {
let spans = Arc::new(Mutex::new(Vec::new()));
let handle = StyleOverlayHandleLua {
spans: Arc::clone(&spans),
};
// Coordinate translation lives on the BUFFER
// (PR #113 round-5 finding 1): buffer-attached
// views see every edit exactly once — Lua bypass
@ -2933,13 +2966,18 @@ fn install_buffer_module(lua: &Lua, registry: &SharedRegistry) -> mlua::Result<T
// attachments below are render-only; per-window
// translation ran once per split and zero times
// hidden.
{
let translator = {
let mut r = reg.borrow_mut();
let buf = resolve_mut(&mut r, id.0)?;
buf.attach_view(Box::new(crate::overlay::BufferStyleSpanTranslator::new(
Arc::clone(&spans),
)));
}
)))
};
let handle = StyleOverlayHandleLua {
spans: Arc::clone(&spans),
buffer: id.0,
translator,
};
attach_style_overlay_to_visible_windows(lua, id.0, &spans);
Ok(handle)
},
@ -2987,7 +3025,12 @@ fn attach_style_overlay_to_visible_windows(
let mut core = core.borrow_mut();
for win in core.windows.values_mut() {
if win.buffer_id == buffer_id {
win.push_overlay(Box::new(crate::overlay::BufferStyleOverlay::new(
// ensure_overlay: idempotent per window via the store's
// identity (round-6 finding 1) — repeated switches into
// the buffer stacked duplicate render views on passive
// panes, each cloning every span and rescanning the
// buffer per frame.
win.ensure_overlay(Box::new(crate::overlay::BufferStyleOverlay::new(
Arc::clone(spans),
)));
}

View File

@ -180,6 +180,16 @@ pub struct BufferStyleSpan {
/// Shared span store used by Lua handles and render overlays.
pub type SharedBufferStyleSpans = Arc<Mutex<Vec<BufferStyleSpan>>>;
/// Identity of a span store: the allocation address, stable for the
/// `Arc`'s lifetime. Every overlay/translator over the same store
/// reports this via [`View::overlay_identity`], which is what makes
/// per-window attachment idempotent and disposal able to find every
/// window copy (PR #113 round-6 findings 1 and 3).
#[must_use]
pub fn style_store_identity(spans: &SharedBufferStyleSpans) -> usize {
Arc::as_ptr(spans) as usize
}
/// View that renders buffer-byte style annotations.
///
/// Unlike [`StyleSpanOverlay`], this overlay stores byte ranges rather
@ -241,6 +251,17 @@ impl View for BufferStyleSpanTranslator {
let old_end = edit.range.end;
let old_len = old_end - old_start;
let new_len = edit.inserted_len;
// Buffers deliberately broadcast no-op edits (empty insert /
// empty-range delete — buffer.rs's "callers that count the
// call" contract). Nothing moved, so there is nothing to
// translate; falling through would split any span containing
// the position into two adjacent fragments per call —
// unbounded growth for repeated no-ops, and a fragment
// boundary mid-codepoint for a no-op at a continuation byte
// (round-6 finding 2).
if old_len == 0 && new_len == 0 {
return Ok(());
}
let mut spans = self.spans.lock().expect("style spans mutex poisoned");
let mut adjusted = Vec::with_capacity(spans.len());
for span in spans.drain(..) {
@ -273,9 +294,25 @@ impl View for BufferStyleSpanTranslator {
fn kind(&self) -> &'static str {
"buffer_style_span_translator"
}
fn overlay_identity(&self) -> Option<usize> {
Some(style_store_identity(&self.spans))
}
}
impl View for BufferStyleOverlay {
fn kind(&self) -> &'static str {
"buffer_style_overlay"
}
fn overlay_identity(&self) -> Option<usize> {
Some(style_store_identity(&self.spans))
}
fn clone_for_split(&self) -> Option<Box<dyn View>> {
Some(Box::new(self.clone()))
}
fn render(&mut self, buf: &Buffer, viewport: Viewport, cells: &mut CellGrid<'_>) {
let spans = self
.spans
@ -850,4 +887,63 @@ mod tests {
"a fully-overwritten span produces no fragments"
);
}
#[test]
fn translator_splits_a_span_around_a_genuine_insertion() {
// A real EditOp::Insert (not a replacement) inside a span:
// the left fragment stays, the right fragment shifts by the
// inserted length, and the inserted bytes inherit nothing.
use crate::buffer::EditOp;
let mut buf = Buffer::from_bytes(BufferId::next(), "t", b"abcdef");
let store: SharedBufferStyleSpans = Arc::new(Mutex::new(vec![BufferStyleSpan {
start: 0,
end: 6,
style: red(),
}]));
buf.attach_view(Box::new(BufferStyleSpanTranslator::new(Arc::clone(&store))));
buf.apply_edit(EditOp::Insert {
pos: 3,
bytes: b"XY",
})
.expect("insert applies");
assert_eq!(
spans_of(&store),
vec![(0, 3), (5, 8)],
"insertion splits the span; inserted bytes are unstyled"
);
}
#[test]
fn translator_ignores_pure_noop_edits() {
// Buffers deliberately broadcast no-op edits (empty insert /
// empty-range delete). PR #113 round-6 finding 2: falling
// through split a containing span into two adjacent
// fragments per call — unbounded growth for repeated no-ops
// at distinct positions, and a fragment boundary
// mid-codepoint for a no-op at a UTF-8 continuation byte.
use crate::buffer::EditOp;
use crate::rope::Range;
let mut buf = Buffer::from_bytes(BufferId::next(), "t", "ab\u{e9}def".as_bytes());
let store: SharedBufferStyleSpans = Arc::new(Mutex::new(vec![BufferStyleSpan {
start: 0,
end: 7,
style: red(),
}]));
buf.attach_view(Box::new(BufferStyleSpanTranslator::new(Arc::clone(&store))));
// Distinct interior positions, including 3 — the é's
// continuation byte.
for pos in [1, 2, 3, 4, 5] {
buf.apply_edit(EditOp::Insert { pos, bytes: b"" })
.expect("no-op insert applies");
buf.apply_edit(EditOp::Delete {
range: Range::new(pos, pos),
})
.expect("no-op delete applies");
}
assert_eq!(
spans_of(&store),
vec![(0, 7)],
"no-op edits must not fragment or move spans"
);
}
}

View File

@ -236,6 +236,27 @@ pub trait View {
fn kind(&self) -> &'static str {
"unknown"
}
/// Identity of the shared resource this overlay renders, if any
/// (PR #113 round-6 findings 1 and 3). Two overlay instances
/// backed by the same store report the same value, which lets a
/// window attach a resource-backed overlay AT MOST once
/// ([`crate::window::Window::ensure_overlay`]) and lets disposal
/// remove every window copy. The default `None` opts out: such
/// views are never deduplicated or bulk-removed.
fn overlay_identity(&self) -> Option<usize> {
None
}
/// A copy of this overlay for a freshly split window showing the
/// same buffer (round-6 finding 1: a same-buffer split starts
/// with an empty overlay list and fires no switch hook, so
/// without this the new pane rendered unstyled). `None` (the
/// default) means the view does not carry across splits;
/// store-backed render overlays return a clone.
fn clone_for_split(&self) -> Option<Box<dyn View>> {
None
}
}
// ---------------------------------------------------------------------------

View File

@ -232,6 +232,25 @@ impl Window {
self.overlays.push(view);
}
/// Push `view` unless an overlay with the same
/// [`View::overlay_identity`] is already attached — attachment
/// of store-backed overlays must be idempotent per window
/// (PR #113 round-6 finding 1: repeated switches into a buffer
/// stacked duplicate render views on passive panes, each cloning
/// every span and rescanning the buffer per frame). Views
/// without an identity always push.
pub fn ensure_overlay(&mut self, view: Box<dyn View>) {
if let Some(id) = view.overlay_identity()
&& self
.overlays
.iter()
.any(|v| v.overlay_identity() == Some(id))
{
return;
}
self.overlays.push(view);
}
/// Stable kind identifiers of every overlay on this window, in
/// push order. Test seam used by `pmacs.window._overlay_kinds()`
/// to verify that a specific overlay type actually attached

View File

@ -2361,6 +2361,158 @@ fn r5f2_partial_rewrite_preserves_untouched_styling() {
);
}
// ---------------------------------------------------------------------------
// PR #113 round 6 — bite tests
// ---------------------------------------------------------------------------
/// Count of `buffer_style_overlay` render views on the ACTIVE window.
fn active_style_overlay_count(s: &EditorState) -> i64 {
eval(
s,
r#"
local n = 0
for _, k in ipairs(pmacs.window._overlay_kinds()) do
if k == "buffer_style_overlay" then n = n + 1 end
end
return n
"#,
)
}
#[test]
fn r6f1_split_panes_stay_styled_with_one_attachment_each() {
use pmacs::cell::Color;
// A styled compilation split into two panes: the split copies
// the render view (splits fire no switch hook and pre-fix left
// the new pane unstyled), and repeated switches of one pane must
// not stack duplicates on the passive pane (pre-fix every
// re-attach blindly pushed to EVERY matching window — unbounded
// render cost). Round-6 finding 1.
let dir = tempfile::tempdir().unwrap();
let script = write_script(
dir.path(),
"red.sh",
"printf '\\033[31mhello\\033[0m world\\n'\n",
);
let mut s = editor();
compile_and_finish(&mut s, &format!("sh {script}"), dir.path());
exec(&s, "pmacs.window.split_horizontal()");
// IMMEDIATELY after the split — before any switch could heal it
// through the attach-to-all-matching-windows path — both panes
// must be styled with one attachment each. This is the split
// half of the finding: pre-fix the new pane had NO render view.
for pane in 0..2 {
assert_eq!(
active_style_overlay_count(&s),
1,
"pane {pane} post-split: exactly one render attachment"
);
let cells = render_active_window_to_grid(&mut s, 12, 60);
let row = styled_row(&cells, 12, 60, "hello world", 5);
assert!(
row.iter().all(|(_, fg)| *fg == Color::Indexed(1)),
"pane {pane} post-split: 'hello' renders red; got {row:?}"
);
exec(&s, "pmacs.window.focus_next()");
}
// Bounce the ACTIVE pane away and back three times; the passive
// pane keeps showing *compilation* through every re-attach. This
// is the accumulation half: pre-fix every re-attach blindly
// pushed another render view onto the passive pane.
exec(&s, r#"bounce_buf = pmacs.buffer.create("*bounce*")"#);
for _ in 0..3 {
exec(&s, "pmacs.window.switch_buffer(bounce_buf)");
exec(
&s,
r#"
for _, id in ipairs(pmacs.buffer.list()) do
if pmacs.describe.buffer(id).name == "*compilation*" then
pmacs.window.switch_buffer(id)
end
end
"#,
);
}
for pane in 0..2 {
assert_eq!(
active_style_overlay_count(&s),
1,
"pane {pane} post-bounce: exactly one render attachment"
);
let cells = render_active_window_to_grid(&mut s, 12, 60);
let row = styled_row(&cells, 12, 60, "hello world", 5);
assert!(
row.iter().all(|(_, fg)| *fg == Color::Indexed(1)),
"pane {pane} post-bounce: 'hello' renders red; got {row:?}"
);
exec(&s, "pmacs.window.focus_next()");
}
}
#[test]
fn r6f2_noop_edits_do_not_fragment_spans() {
// Buffers deliberately broadcast no-op edits; the translator
// must ignore them (round-6 finding 2). Pre-fix each interior
// no-op split the containing span into two adjacent fragments —
// unbounded growth, and position 3 (the é's continuation byte)
// minted a mid-codepoint span boundary.
let s = editor();
let (count, start, end): (i64, i64, i64) = eval(
&s,
r#"
local buf = pmacs.buffer.create("*noop-spans*")
local ov = pmacs.buffer.add_style_overlay(buf)
buf:insert(0, "ab\195\169def")
ov:add(0, 7, { fg = 1 })
for _, pos in ipairs({ 1, 2, 3, 4, 5 }) do
buf:insert(pos, "")
buf:delete(pos, pos)
end
local spans = ov:spans()
return #spans, spans[1].start, spans[1]["end"]
"#,
);
assert_eq!(
(count, start, end),
(1, 0, 7),
"no-op edits must neither fragment nor move spans"
);
}
#[test]
fn r6f3_handle_dispose_detaches_translator_and_render_views() {
// Teardown path (round-6 finding 3): dispose() detaches the
// buffer-attached translator (later edits stop translating) and
// removes every window render view over the handle's store;
// calling it twice is safe. Pre-fix a dropped handle's
// translator lived until the buffer died.
let s = editor();
let (live, stale, render_views): (i64, i64, i64) = eval(
&s,
r#"
local buf = pmacs.buffer.create("*disposable*")
pmacs.window.switch_buffer(buf)
local ov = pmacs.buffer.add_style_overlay(buf)
buf:insert(0, "abcdef")
ov:add(0, 6, { fg = 1 })
buf:insert(0, "xx") -- live translator: span shifts
local live = ov:spans()[1].start
ov:dispose()
buf:insert(0, "yy") -- detached: span must NOT move
local stale = ov:spans()[1].start
ov:dispose() -- idempotent
local n = 0
for _, k in ipairs(pmacs.window._overlay_kinds()) do
if k == "buffer_style_overlay" then n = n + 1 end
end
return live, stale, n
"#,
);
assert_eq!(live, 2, "pre-dispose edits translate the span");
assert_eq!(stale, 2, "post-dispose edits must not reach the store");
assert_eq!(render_views, 0, "dispose removes the window render views");
}
#[test]
fn r5f3_tracked_line_start_matches_the_scan_across_transitions() {
// Round-5 finding 3 is a performance fix — the per-CR/BS/erase