fix(themes): PR #120 round 3 -- GPU snapshot symmetry, count freeze
Finding 1: the round-2 reset contract was asymmetric. The producer resets search/menu/status baselines on every snapshot send, but the GPU's BufferSnapshot arm only cleared spans, decorations, adornments, summary, and the completion popup -- a menu or search open at switch time survived the snapshot with no close message ever coming (the new buffer's first CLOSED state is suppressed daemon-side), leaving a stale popup that also held daemon_intercepts_keys true and swallowed pointer events indefinitely. The arm now clears search_prompt, menu, and status_facts; the minibuffer is deliberately exempt on both sides (one global core instance, matching the producer's surviving last_minibuffer baseline). GPU test opens search + menu + status via the real wire arms, applies a snapshot, and asserts all three clear, the intercept gate releases, and the popup pixels vanish -- hand-bitten by disabling the three clears (fix and test share main.rs). Finding 2: the round-2 reset broke the diagnostic-count freeze. last_status was both the peer emission baseline and the stale-store freeze source, so a snapshot between didChange and fresh diagnostics re-shipped StatusFacts with zeroed counts. The freeze source now lives apart: frozen_diag_counts advances on every fresh count, is read when the store is stale, and survives on_buffer_snapshot_sent -- which keeps killing the emission baseline to force the re-send. Acceptance renders (1,1), marks the store stale, applies the reset, and asserts the re-sent StatusFacts still carries (1,1); runtime bite vs pre-fix semantic_render.rs fails exactly as predicted (Some((0,0)) vs Some((1,1))). Framing revision 7; acceptance items 31-32; the protocol doc's snapshot-reset paragraph now lists the full frontend drop set and names the count freeze as daemon knowledge, not peer state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VoiEyuPjoBhvwACf8HAnLB
This commit is contained in:
parent
d91ff1a9e2
commit
2fe6738d68
|
|
@ -119,14 +119,18 @@ variants, and one frontend→instance `Viewport` variant.
|
|||
**`BufferSnapshot` resets buffer-scoped interpretation state.** A
|
||||
frontend receiving a snapshot drops everything it holds for the
|
||||
named buffer — spans, decorations, adornments, minimap summary,
|
||||
completion popup — and rebuilds from the frames that follow; the
|
||||
instance mirrors this by invalidating its per-buffer emission
|
||||
baselines whenever it writes a snapshot, so the frontend's
|
||||
post-snapshot viewport declaration receives authoritative re-sends
|
||||
even when nothing changed daemon-side (the unchanged-generation
|
||||
A → B → A revisit). Bufferless facts (`ThemeFacts`, the minibuffer
|
||||
prompt) and per-frontend state (the gutter mode) survive snapshots
|
||||
on both sides.
|
||||
completion popup, search and menu prompts (which also gate the
|
||||
frontend's key/pointer interception), and status facts — and
|
||||
rebuilds from the frames that follow; the instance mirrors this by
|
||||
invalidating its per-buffer emission baselines whenever it writes a
|
||||
snapshot, so the frontend's post-snapshot viewport declaration
|
||||
receives authoritative re-sends even when nothing changed
|
||||
daemon-side (the unchanged-generation A → B → A revisit). Bufferless
|
||||
facts (`ThemeFacts`, the minibuffer prompt) and per-frontend state
|
||||
(the gutter mode) survive snapshots on both sides, and the
|
||||
instance's stale-store diagnostic-count freeze is daemon knowledge,
|
||||
not peer state — the re-sent `StatusFacts` after a snapshot carries
|
||||
the frozen counts, never zeros.
|
||||
|
||||
## Capability and version mechanics
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,29 @@
|
|||
# Theme faces — framing (Arc 4 stage 1, themes)
|
||||
|
||||
**Revision 6 — 2026-07-15. Status: implemented on branch
|
||||
`theme-faces` (PR #120); revision 6 folds PR round 2.**
|
||||
**Revision 7 — 2026-07-15. Status: implemented on branch
|
||||
`theme-faces` (PR #120); revision 7 folds PR round 3.**
|
||||
|
||||
Revision 7 (PR #120 round 3, findings 1–2): the reset contract is
|
||||
now symmetric on the GPU. The round-2 producer reset covered
|
||||
search/menu/status baselines, but the GPU's `BufferSnapshot` arm
|
||||
only cleared spans, decorations, adornments, summary, and the
|
||||
completion popup — a menu (or search) open at switch time survived
|
||||
the snapshot with no close message ever coming (the new buffer's
|
||||
first CLOSED state is suppressed daemon-side), leaving a stale
|
||||
popup that also gated key and pointer interception
|
||||
(`daemon_intercepts_keys`) indefinitely. The arm now clears
|
||||
`search_prompt`, `menu`, and `status_facts`; the minibuffer is
|
||||
deliberately exempt (one global core instance, matching the
|
||||
producer's surviving `last_minibuffer` baseline) (finding 1). And
|
||||
the round-2 reset broke the diagnostic-count freeze: `last_status`
|
||||
was both the peer emission baseline and the stale-store freeze
|
||||
source, so a snapshot between didChange and fresh diagnostics
|
||||
re-shipped `StatusFacts` with zeroed counts. The freeze source now
|
||||
lives apart — `frozen_diag_counts`, daemon-side knowledge about the
|
||||
buffer, advanced on every fresh count and read when the store is
|
||||
stale — and survives `on_buffer_snapshot_sent`, which keeps killing
|
||||
the emission baseline to force the re-send (finding 2). Acceptance
|
||||
items 31–32.
|
||||
|
||||
Revision 6 (PR #120 round 2, findings 1–2): the producer gains the
|
||||
**snapshot/baseline reset contract**
|
||||
|
|
@ -627,10 +649,19 @@ pub struct ThemeFace {
|
|||
needs no call. NOT reset: `last_face_epoch` / `last_theme_faces`
|
||||
(this channel is bufferless — the frontend keeps its face table
|
||||
across snapshots), the global minibuffer baseline, the
|
||||
per-frontend gutter mode, the revision-keyed diag line cache, and
|
||||
other buffers' baselines (each buffer's own snapshot precedes its
|
||||
revisit). Resetting on a failed write is harmless — the failure
|
||||
mode is one redundant re-send, never staleness.
|
||||
per-frontend gutter mode, the revision-keyed diag line cache,
|
||||
`frozen_diag_counts` (round 3 finding 2: the stale-store freeze
|
||||
source is daemon-side knowledge about the buffer, split from the
|
||||
`last_status` peer baseline precisely so the reset cannot zero
|
||||
mid-edit counts), and other buffers' baselines (each buffer's own
|
||||
snapshot precedes its revisit). Resetting on a failed write is
|
||||
harmless — the failure mode is one redundant re-send, never
|
||||
staleness. The GPU's `BufferSnapshot` arm mirrors the contract
|
||||
(round 3 finding 1): it clears its buffer-scoped facts —
|
||||
search/menu popups (which gate key and pointer interception) and
|
||||
the status band — alongside spans, decorations, adornments,
|
||||
summary, and the completion popup; the minibuffer survives on
|
||||
both sides.
|
||||
- `docs/semantic-frontend-protocol.md` gains the channel's contract
|
||||
section.
|
||||
|
||||
|
|
@ -951,3 +982,13 @@ Keybinding-driven tests dispatch keys, never `pmacs.command.invoke`.
|
|||
round trip (same text both buffers, isolating the minimap), the
|
||||
stale minimap is gone, and applying the re-shipped summary at
|
||||
the same generation restores the first visit's pixels exactly.
|
||||
31. **Snapshots clear GPU search/menu/status and release the
|
||||
intercept gate** (PR round 3 finding 1, `PMACS_REQUIRE_GPU=1`):
|
||||
with an open search prompt, menu popup, and status facts,
|
||||
applying a `BufferSnapshot` clears all three, flips
|
||||
`daemon_intercepts_keys` back to local key handling, and removes
|
||||
the popup pixels — hand-bitten by disabling the three clears.
|
||||
32. **The reset preserves the diagnostic-count freeze** (PR round 3
|
||||
finding 2): render nonzero counts, mark the store stale
|
||||
(didChange), apply the snapshot reset — the re-sent
|
||||
`StatusFacts` carries the frozen nonzero counts, never (0, 0).
|
||||
|
|
|
|||
|
|
@ -2465,6 +2465,22 @@ impl State {
|
|||
// keep hijacking Esc/RET/TAB. The daemon-side session
|
||||
// was already invalidated by the switch.
|
||||
self.completion = None;
|
||||
// PR #120 round 3 finding 1 — the remaining
|
||||
// buffer-scoped facts, same reasoning: search and menu
|
||||
// popups anchor in the prior buffer AND gate key and
|
||||
// pointer interception (`daemon_intercepts_keys`, the
|
||||
// pointer arms), and the new buffer's first CLOSED
|
||||
// state is suppressed daemon-side, so no close message
|
||||
// ever comes — a retained popup would hijack input
|
||||
// forever. The status band's name/counts describe the
|
||||
// buffer we just left; the producer's reset contract
|
||||
// re-ships the new buffer's facts on its first frame.
|
||||
// The minibuffer deliberately survives: it is one
|
||||
// global core instance, matching the producer's
|
||||
// surviving `last_minibuffer` baseline.
|
||||
self.search_prompt = None;
|
||||
self.menu = None;
|
||||
self.status_facts = None;
|
||||
self.cursor_fresh = false;
|
||||
self.optimistic_cursor_floor = None;
|
||||
self.optimistic_floor_set_at = None;
|
||||
|
|
@ -8777,6 +8793,80 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn headless_snapshot_clears_search_menu_status_and_the_intercept_gate() {
|
||||
// PR #120 round 3 finding 1: search/menu popups anchor in the
|
||||
// prior buffer AND gate key/pointer interception, and the new
|
||||
// buffer's first CLOSED state is suppressed daemon-side, so
|
||||
// no close message ever comes — a `BufferSnapshot` must clear
|
||||
// them (and the stale status band) or the popup hijacks input
|
||||
// forever. The minibuffer is global and deliberately exempt.
|
||||
let text = "alpha\nbeta\n";
|
||||
let Some(mut state) = headless_or_skip(400, 300, text) else {
|
||||
return;
|
||||
};
|
||||
let bid_a = BufferId::next();
|
||||
state.current_buffer_id = Some(bid_a);
|
||||
let _ = state.apply_attach_message(InstanceMessage::DispatchIdle { idle: true });
|
||||
assert!(
|
||||
!state.daemon_intercepts_keys(),
|
||||
"idle with nothing open: keys apply locally"
|
||||
);
|
||||
|
||||
let _ = state.apply_attach_message(InstanceMessage::SearchPrompt {
|
||||
buffer_id: bid_a,
|
||||
query: Some("al".into()),
|
||||
active: Some(0),
|
||||
total: 1,
|
||||
regex: false,
|
||||
invalid: false,
|
||||
});
|
||||
let _ = state.apply_attach_message(InstanceMessage::MenuPrompt {
|
||||
buffer_id: bid_a,
|
||||
rows: vec![MenuPromptRow {
|
||||
label: "Cut".into(),
|
||||
separator: false,
|
||||
}],
|
||||
active: Some(0),
|
||||
});
|
||||
let _ = state.apply_attach_message(InstanceMessage::StatusFacts {
|
||||
buffer_id: bid_a,
|
||||
name: "old.rs".into(),
|
||||
modified: false,
|
||||
diag_errors: 3,
|
||||
diag_warnings: 1,
|
||||
message: None,
|
||||
});
|
||||
assert!(
|
||||
state.daemon_intercepts_keys(),
|
||||
"an open search/menu round-trips every key"
|
||||
);
|
||||
let with_popups = state.render_offscreen();
|
||||
|
||||
let doc = loro::LoroDoc::new();
|
||||
doc.get_text(LORO_TEXT_CONTAINER)
|
||||
.insert(0, text)
|
||||
.expect("insert snapshot text");
|
||||
let _ = state.apply_attach_message(InstanceMessage::BufferSnapshot {
|
||||
buffer_id: BufferId::next(),
|
||||
crdt_snapshot: doc.export(loro::ExportMode::Snapshot).expect("export"),
|
||||
});
|
||||
|
||||
assert!(
|
||||
state.search_prompt.is_none() && state.menu.is_none() && state.status_facts.is_none(),
|
||||
"buffer-scoped search/menu/status facts die with the snapshot"
|
||||
);
|
||||
assert!(
|
||||
!state.daemon_intercepts_keys(),
|
||||
"the intercept gate releases — keys apply locally again"
|
||||
);
|
||||
assert_ne!(
|
||||
with_popups,
|
||||
state.render_offscreen(),
|
||||
"the popup pixels are gone"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn own_wash_faces_color_local_rects_peers_keep_the_constant() {
|
||||
// Acceptance 21 + 23: decode the emitted decoration vertex
|
||||
|
|
|
|||
|
|
@ -177,6 +177,16 @@ pub struct SemanticRenderState {
|
|||
/// emitted as `StatusFacts` (Q#S1; `message` since v15) —
|
||||
/// cached-compare suppression.
|
||||
last_status: HashMap<BufferId, (String, bool, u32, u32, Option<String>)>,
|
||||
/// Last *fresh* whole-file diagnostic counts per buffer — the
|
||||
/// freeze source while the diag store is stale (mid-edit counts
|
||||
/// merely lag; flickering to zero per keystroke would be worse).
|
||||
/// Split from `last_status` (PR #120 round 3 finding 2): that map
|
||||
/// is a PEER emission baseline and dies with a `BufferSnapshot`
|
||||
/// (`on_buffer_snapshot_sent`), while this is daemon-side
|
||||
/// knowledge about the buffer and must survive the reset — or a
|
||||
/// buffer switch between didChange and fresh diagnostics would
|
||||
/// re-ship `StatusFacts` with zeroed counts.
|
||||
frozen_diag_counts: HashMap<BufferId, (u32, u32)>,
|
||||
/// Last-emitted line-number gutter mode (UX gutter arc, protocol v14) —
|
||||
/// cached-compare suppression. Seeded to `Some(Off)` (the frontend's
|
||||
/// default) so an off gutter never emits. Per-frontend (one value),
|
||||
|
|
@ -352,6 +362,7 @@ impl SemanticRenderState {
|
|||
last_completion_popup: HashMap::new(),
|
||||
last_summary: HashMap::new(),
|
||||
last_status: HashMap::new(),
|
||||
frozen_diag_counts: HashMap::new(),
|
||||
// Seed to the frontend's default (gutter off): a plain default
|
||||
// window never emits `LineNumbers`, so the common case adds no
|
||||
// traffic and the first frame is unchanged. Only an actual
|
||||
|
|
@ -403,10 +414,14 @@ impl SemanticRenderState {
|
|||
/// table across snapshots), `last_minibuffer` (one global core
|
||||
/// instance, not buffer-scoped), `last_line_numbers`
|
||||
/// (per-frontend gutter mode, kept by the frontend across the
|
||||
/// switch), and `diag_line_cache` (a revision-keyed compute
|
||||
/// cache, not a peer-state baseline). Baselines for OTHER buffers
|
||||
/// also survive — the snapshot names one buffer, and any buffer
|
||||
/// the frontend navigates to receives its own snapshot first.
|
||||
/// switch), `diag_line_cache` (a revision-keyed compute cache,
|
||||
/// not a peer-state baseline), and `frozen_diag_counts` (the
|
||||
/// stale-store freeze source — daemon-side knowledge about the
|
||||
/// buffer, not about the peer; deleting it would zero the counts
|
||||
/// on a mid-edit revisit, round 3 finding 2). Baselines for OTHER
|
||||
/// buffers also survive — the snapshot names one buffer, and any
|
||||
/// buffer the frontend navigates to receives its own snapshot
|
||||
/// first.
|
||||
pub fn on_buffer_snapshot_sent(&mut self, buffer_id: BufferId) {
|
||||
self.last_sent.remove(&buffer_id);
|
||||
self.last_style_gate.remove(&buffer_id);
|
||||
|
|
@ -919,11 +934,24 @@ impl SemanticRenderState {
|
|||
}
|
||||
})
|
||||
};
|
||||
let cached = self.last_status.get(&buffer_id);
|
||||
let (diag_errors, diag_warnings) =
|
||||
counts.unwrap_or_else(|| cached.map_or((0, 0), |c| (c.2, c.3)));
|
||||
// Fresh counts advance the freeze source; a stale store reads
|
||||
// it back. Never `last_status` — that is the peer emission
|
||||
// baseline and dies with a `BufferSnapshot`, while the frozen
|
||||
// counts are daemon-side knowledge that must survive the
|
||||
// reset (round 3 finding 2).
|
||||
let (diag_errors, diag_warnings) = match counts {
|
||||
Some(fresh) => {
|
||||
self.frozen_diag_counts.insert(buffer_id, fresh);
|
||||
fresh
|
||||
}
|
||||
None => self
|
||||
.frozen_diag_counts
|
||||
.get(&buffer_id)
|
||||
.copied()
|
||||
.unwrap_or((0, 0)),
|
||||
};
|
||||
let facts = (name, modified, diag_errors, diag_warnings, message);
|
||||
if cached == Some(&facts) {
|
||||
if self.last_status.get(&buffer_id) == Some(&facts) {
|
||||
return None;
|
||||
}
|
||||
let msg = InstanceMessage::StatusFacts {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
// theme_faces_acceptance.rs --- Themes Arc 4 stage 1 acceptance
|
||||
// (docs/theme-faces-framing.md, acceptance items 1–19, 24–26, and
|
||||
// 28–29; the GPU routes — 20–23, 27, and 30 — live in pmacs-gpu's
|
||||
// (docs/theme-faces-framing.md, acceptance items 1–19, 24–26, 28–29,
|
||||
// and 32; the GPU routes — 20–23, 27, and 30–31 — live in pmacs-gpu's
|
||||
// headless suite).
|
||||
|
||||
//! Named UI faces (`ui` / `ui.*` theme entries) + the `ThemeFacts`
|
||||
|
|
@ -1074,6 +1074,78 @@ fn daemon_reships_the_summary_after_a_real_buffer_round_trip() {
|
|||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 32 — the snapshot reset must not break the diagnostic-count freeze
|
||||
// (PR #120 round 3 finding 2)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
#[test]
|
||||
fn snapshot_reset_keeps_frozen_diag_counts_while_the_store_is_stale() {
|
||||
// `last_status` was both the peer emission baseline AND the
|
||||
// stale-store freeze source, so the round-2 reset zeroed the
|
||||
// counts: a buffer switch between didChange and fresh
|
||||
// diagnostics re-shipped StatusFacts with (0, 0). The freeze
|
||||
// source now lives apart (`frozen_diag_counts`) and survives
|
||||
// `on_buffer_snapshot_sent`.
|
||||
use pmacs::diag::DiagnosticSeverity;
|
||||
|
||||
let mut state = editor();
|
||||
type_str(&mut state, "boom\nfine\n");
|
||||
let uri = attach_diags(
|
||||
&state,
|
||||
vec![
|
||||
diag(DiagnosticSeverity::Error),
|
||||
diag(DiagnosticSeverity::Warning),
|
||||
],
|
||||
);
|
||||
|
||||
let status_counts = |msgs: &[InstanceMessage]| {
|
||||
msgs.iter().find_map(|m| match m {
|
||||
InstanceMessage::StatusFacts {
|
||||
diag_errors,
|
||||
diag_warnings,
|
||||
..
|
||||
} => Some((*diag_errors, *diag_warnings)),
|
||||
_ => None,
|
||||
})
|
||||
};
|
||||
|
||||
let a = active_buffer(&state);
|
||||
let mut sem = semantic(&state);
|
||||
assert_eq!(
|
||||
status_counts(&sem.render_frame(&state)),
|
||||
Some((1, 1)),
|
||||
"fresh counts ship on the first frame"
|
||||
);
|
||||
|
||||
// didChange: the store goes stale — counts freeze, never zero.
|
||||
{
|
||||
let store = state.lsp_manager.borrow().diag_store();
|
||||
store
|
||||
.lock()
|
||||
.expect("diag store lock")
|
||||
.mark_stale(uri.clone());
|
||||
}
|
||||
|
||||
// The daemon writes snapshot(A) mid-staleness (a revisit, or the
|
||||
// F29 upgrade broadcast). The reset kills the emission baseline —
|
||||
// forcing the re-send — but must not discard the freeze source.
|
||||
sem.on_buffer_snapshot_sent(a);
|
||||
sem.set_viewport(
|
||||
a,
|
||||
ByteRange {
|
||||
start: 0,
|
||||
end: 1 << 20,
|
||||
},
|
||||
0,
|
||||
);
|
||||
assert_eq!(
|
||||
status_counts(&sem.render_frame(&state)),
|
||||
Some((1, 1)),
|
||||
"the re-sent StatusFacts preserves the frozen counts"
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 12 + 16 + 19 — ThemeFacts emission discipline; late join; set wipes
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Reference in New Issue