Keep the plot origin when a render is held past source()

crates/ark/src/console/console_graphics.rs

@@ -39,6 +39,5 @@ impl Console {
         let dc = Rc::clone(self.device_context());
         dc.process_changes(self);
         dc.clear_execution_context();
-        dc.clear_pending_origin();
     }
 }

crates/ark/src/plots/graphics_device.rs

@@ -187,10 +187,10 @@ pub(crate) struct DeviceContext {
     /// provides the file attribution even though the execute_request came from the console.
     source_context_stack: RefCell<Vec<String>>,
 
-    /// The plot origin captured eagerly when drawing starts (i.e. when `has_changes`
-    /// transitions from false to true). This is necessary because the source context
-    /// stack may be popped before `process_changes()` runs (e.g. `source()` completes
-    /// before the execute request finishes), so we snapshot the origin at drawing time.
+    /// The plot origin for the current page, captured eagerly whenever drawing
+    /// starts and no snapshot is pending. This is necessary because the source
+    /// context stack may be popped before `process_changes()` runs (e.g. `source()`
+    /// completes before the execute request finishes).
     pending_origin: RefCell<Option<Option<PlotOrigin>>>,
 }
 
@@ -269,15 +269,14 @@ impl DeviceContext {
         self.source_context_stack.borrow().last().cloned()
     }
 
-    /// Eagerly capture the plot origin so it's available when `process_changes()` runs later.
-    /// Called when drawing first starts for a change set, since the source context stack
-    /// may be popped before we get a chance to consume it.
+    /// Eagerly capture the plot origin so it's available when `process_changes()`
+    /// runs later, since the source context stack may be popped before then.
     fn set_pending_origin(&self, origin: Option<PlotOrigin>) {
         self.pending_origin.replace(Some(origin));
     }
 
     /// Clear any unconsumed pending origin.
-    pub(crate) fn clear_pending_origin(&self) {
+    fn clear_pending_origin(&self) {
         self.pending_origin.replace(None);
     }
 
@@ -338,10 +337,13 @@ impl DeviceContext {
         let old_has_changes = self.has_changes.get();
         self.has_changes.replace(old_has_changes || is_drawing);
 
-        // Eagerly capture the plot origin when drawing first starts for this
-        // change set. The source context stack may be popped before
-        // `process_changes()` runs, so we snapshot it now while it's available.
-        if !old_has_changes && is_drawing {
+        // Eagerly capture the plot origin while the source context stack is still
+        // available, since `process_changes()` may not run until after `source()`
+        // popped it. Capture whenever we have no snapshot rather than only on the
+        // `has_changes` false->true edge, which never comes back around for a new
+        // page started while earlier changes are still pending (e.g. `dev.hold()`).
+        let needs_origin = self.pending_origin.borrow().is_none();
+        if is_drawing && needs_origin {
             let ctx = self.capture_execution_context();
             let origin = self.capture_plot_origin(&ctx);
             self.set_pending_origin(origin);

crates/ark/tests/integration/plots.rs

@@ -585,6 +585,52 @@ fn test_plot_source_context_stacking() {
     assert!(result_a.contains(&format!("$origin_uri\n[1] \"{}\"", file_a.uri_id)));
 }
 
+/// Test that a plot created inside `source()` keeps its origin when rendering
+/// is held past the end of the `source()` call.
+///
+/// https://github.com/posit-dev/ark/issues/1334
+///
+/// While a hold is active, `process_changes()` deliberately leaves
+/// `has_changes` set so it can notify once the hold is released. That leaves
+/// `has_changes` already true when the next plot starts drawing, which skips
+/// the eager origin capture in `hook_mode()`. By the time the hold is released,
+/// `source()` has returned and its `defer()` has popped the source context, so
+/// the origin has nothing left to fall back to.
+#[test]
+fn test_plot_origin_survives_hold_across_source() {
+    let frontend = DummyArkFrontend::lock();
+
+    // Two plots under a hold, with no `dev.flush()`, so the notification is
+    // deferred past the end of `source()`.
+    let file = SourceFile::new("invisible(dev.hold())\nplot(1:5)\nplot(1:3)\n");
+
+    let code = format!("source('{}')", file.path);
+    frontend.send_execute_request(&code, ExecuteRequestOptions::default());
+    frontend.recv_iopub_busy();
+    frontend.recv_iopub_execute_input();
+    frontend.recv_iopub_idle();
+    frontend.recv_shell_execute_reply();
+
+    // Release the hold in a separate request, after source() has returned.
+    frontend.send_execute_request("invisible(dev.flush())", ExecuteRequestOptions::default());
+    frontend.recv_iopub_busy();
+    frontend.recv_iopub_execute_input();
+    let display_id = frontend.recv_iopub_display_data_id();
+    assert!(!display_id.is_empty());
+    frontend.recv_iopub_idle();
+    frontend.recv_shell_execute_reply();
+
+    let query = format!(".ps.graphics.get_metadata('{display_id}')");
+    frontend.send_execute_request(&query, ExecuteRequestOptions::default());
+    frontend.recv_iopub_busy();
+    frontend.recv_iopub_execute_input();
+    let result = frontend.recv_iopub_execute_result();
+    frontend.recv_iopub_idle();
+    frontend.recv_shell_execute_reply();
+
+    assert!(result.contains(&format!("$origin_uri\n[1] \"{}\"", file.uri_id)));
+}
+
 /// Test that plots rendered with fig-width/fig-height metadata produce
 /// a PNG at the expected pixel dimensions (inches * 96 DPI).
 #[test]