From d6f6cfaae1c38951dc42f1a32b357f2f7e29b150 Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Sat, 11 Jul 2026 00:25:44 +0800 Subject: [PATCH] fix(editor): bump document revision on raw content mutations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The layer-panel row cache keys on document_revision, which made revision correctness load-bearing: host paths that mutate content through raw active_children_mut() without mark_document_changed() left the panel painting stale rows (and is_dirty save-tracking wrong) — most visibly the starter-frame clear before a design run. Adds the bump at the four raw sites (starter clear, image-search apply, both pick-fill handlers); all other raw-mutation sites audited as already bumping via history. --- .../src/chat_session_launch.rs | 44 +++++++++++++++++++ .../src/image_search_session.rs | 22 +++++++--- .../src/image_search_session_tests.rs | 8 ++++ .../op-host-desktop/src/persistence_image.rs | 8 +++- crates/op-host-web/src/dom_io.rs | 13 ++++-- 5 files changed, 86 insertions(+), 9 deletions(-) diff --git a/crates/op-host-desktop/src/chat_session_launch.rs b/crates/op-host-desktop/src/chat_session_launch.rs index 803933818..40ed5d7a3 100644 --- a/crates/op-host-desktop/src/chat_session_launch.rs +++ b/crates/op-host-desktop/src/chat_session_launch.rs @@ -484,6 +484,11 @@ pub(crate) fn clear_fresh_starter_frame_for_design(state: &mut EditorState) -> b } state.active_children_mut().clear(); state.clear_selection(); + // Raw `active_children_mut()` bypasses the command/history path, so + // bump the document revision explicitly. Without it the layer-panel + // row cache (keyed on `document_revision()`) keeps painting the + // now-deleted starter "Frame" row, and save-dirty tracking stays wrong. + state.mark_document_changed(); true } @@ -767,6 +772,45 @@ mod tests { node } + #[test] + fn clear_fresh_starter_frame_bumps_document_revision() { + let mut state = EditorState::new(); + // Install the exact blank starter frame the design classifier + // recognizes (id "n10", name "Frame", 1200x800, white fill). + let starter: jian_ops_schema::node::PenNode = serde_json::from_value(serde_json::json!({ + "type": "frame", + "id": "n10", + "name": "Frame", + "x": 0, + "y": 0, + "width": 1200, + "height": 800, + "fill": [{ "type": "solid", "color": "#ffffff" }], + "children": [] + })) + .expect("starter frame fixture"); + state.active_children_mut().clear(); + state.active_children_mut().push(starter); + let revision_before = state.document_revision(); + + assert!( + clear_fresh_starter_frame_for_design(&mut state), + "the blank starter frame must be recognized and cleared" + ); + assert!( + state.active_children().is_empty(), + "the starter Frame row must be gone after the clear" + ); + // Regression: the raw `active_children_mut().clear()` must bump the + // revision, or the layer-panel row cache (keyed on + // `document_revision()`) keeps painting the deleted "Frame" row. + assert_ne!( + state.document_revision(), + revision_before, + "clearing the starter frame must advance document_revision" + ); + } + #[test] fn builtin_design_keyword_with_existing_target_prefers_modify_route() { let mut state = EditorState::new(); diff --git a/crates/op-host-desktop/src/image_search_session.rs b/crates/op-host-desktop/src/image_search_session.rs index fb12a3aa8..4faed4154 100644 --- a/crates/op-host-desktop/src/image_search_session.rs +++ b/crates/op-host-desktop/src/image_search_session.rs @@ -292,10 +292,13 @@ impl ImageSearchSession { let job = self.jobs.swap_remove(i); let id = job.node_id.as_str().to_string(); self.in_flight.remove(&id); - // `in_flight`/`completed` mutated outside `enqueue_missing` - // (and `apply_result` may edit the document without bumping - // its revision) — invalidate the scan gate so the next - // `enqueue_missing` re-walks once. + // `in_flight`/`completed` are mutated outside + // `enqueue_missing`, and a failed job updates them without + // any document-content change (so no revision bump) — + // invalidate the scan gate so the next `enqueue_missing` + // re-walks once. (A successful `apply_result` DOES bump the + // revision, but the gate invalidation still covers the + // failure path.) self.last_scanned = None; if let Some(url) = url { if apply_result(state, &job.node_id, &url) { @@ -724,7 +727,7 @@ pub(crate) fn apply_result(state: &mut EditorState, node_id: &NodeId, url: &str) }; let is_unfilled_placeholder_frame = is_frame_placeholder_still_unfilled(node); let is_unfilled_placeholder_rectangle = is_image_area_rectangle_by_heuristic(node); - match node { + let changed = match node { PenNode::Image(image) => { if image.src == url { return false; @@ -771,7 +774,16 @@ pub(crate) fn apply_result(state: &mut EditorState, node_id: &NodeId, url: &str) true } _ => false, + }; + if changed { + // This writes document content through raw `active_children_mut()` + // outside the command/history path, so bump the revision. The + // layer-panel row cache + save-dirty tracking key on + // `document_revision()`; the placeholder-frame/rectangle branches + // also clear `children`, which changes the visible layer rows. + state.mark_document_changed(); } + changed } fn fetch_first_image_url_blocking( diff --git a/crates/op-host-desktop/src/image_search_session_tests.rs b/crates/op-host-desktop/src/image_search_session_tests.rs index 0accd664f..54fb3f55e 100644 --- a/crates/op-host-desktop/src/image_search_session_tests.rs +++ b/crates/op-host-desktop/src/image_search_session_tests.rs @@ -234,6 +234,7 @@ fn apply_result_sets_empty_image_src() { state .active_children_mut() .push(image_node("img1", "", Some("burger fries"))); + let revision_before = state.document_revision(); assert!(apply_result( &mut state, @@ -244,6 +245,13 @@ fn apply_result_sets_empty_image_src() { panic!("expected image"); }; assert_eq!(image.src, "https://example.com/photo.jpg"); + // A content-mutating apply_result bumps the revision so the layer-panel + // row cache + save-dirty tracking (keyed on `document_revision()`) refresh. + assert_ne!( + state.document_revision(), + revision_before, + "apply_result that writes content must advance document_revision" + ); } #[test] diff --git a/crates/op-host-desktop/src/persistence_image.rs b/crates/op-host-desktop/src/persistence_image.rs index 748c86409..3f97f6891 100644 --- a/crates/op-host-desktop/src/persistence_image.rs +++ b/crates/op-host-desktop/src/persistence_image.rs @@ -143,7 +143,13 @@ pub fn handle_pick_fill_image(host: &mut WidgetHostNative) { return; } }; - let _ = host.editor_state_mut().set_selected_fill_image_url(&url); + if host.editor_state_mut().set_selected_fill_image_url(&url) { + // `set_selected_fill_image_url` writes fill content without touching + // the command/history path, so bump the revision (layer-panel cache + + // save-dirty tracking key on `document_revision()`). The sibling + // relink handler above bumps via `commit_history()`. + host.editor_state_mut().mark_document_changed(); + } host.mark_editor_state_dirty(); } diff --git a/crates/op-host-web/src/dom_io.rs b/crates/op-host-web/src/dom_io.rs index 017afac26..51249b18d 100644 --- a/crates/op-host-web/src/dom_io.rs +++ b/crates/op-host-web/src/dom_io.rs @@ -623,10 +623,17 @@ fn pick_fill_image(inner: &InnerRc) { return; }; let mut b = inner2.borrow_mut(); - let _ = b - .host_mut() + if b.host_mut() .editor_state_mut() - .set_selected_fill_image_url(&url); + .set_selected_fill_image_url(&url) + { + // Fill content written outside the command/history + // path — bump the revision so the layer-panel cache + + // save-dirty tracking (keyed on `document_revision()`) + // see it. The relink handler below bumps via + // `commit_history()`. + b.host_mut().editor_state_mut().mark_document_changed(); + } b.host_mut().mark_editor_state_dirty(); let _ = b.repaint(); }),