From 31fe263653684adee8024acb6cee005883663a7d Mon Sep 17 00:00:00 2001 From: Thomas Lin Pedersen Date: Thu, 23 Apr 2026 11:20:51 +0200 Subject: [PATCH 1/3] differentiate between plot pane/inline/ and regular output in the kernel --- ggsql-jupyter/src/display.rs | 304 +++++++++++++++++++++++++++++++---- ggsql-jupyter/src/kernel.rs | 13 +- 2 files changed, 284 insertions(+), 33 deletions(-) diff --git a/ggsql-jupyter/src/display.rs b/ggsql-jupyter/src/display.rs index 8de08818..d41a1f25 100644 --- a/ggsql-jupyter/src/display.rs +++ b/ggsql-jupyter/src/display.rs @@ -4,9 +4,54 @@ //! with appropriate MIME types for rich rendering. use crate::executor::ExecutionResult; +use crate::message::MessageHeader; use polars::frame::DataFrame; use serde_json::{json, Value}; +/// Frontend-supplied hints about the output rendering slot. +/// +/// Three render targets, identified by the Jupyter session id on the +/// incoming execute_request: +/// +/// - **Positron notebook** (`ggsql-notebook-…`): inline code-chunk output +/// in an editor view. Rendered into a plain 400px container with no +/// layout-mutating observers, because Positron animates the slot during +/// its reveal transition. +/// - **Positron console** (`ggsql-…`): output lands in the Plots pane. The +/// container upgrades to `100vh` inside `.positron-output-container`, so +/// Vega-Lite's own container observer tracks pane resizes. +/// - **Standalone** (anything else — Jupyter notebook, Quarto render, …): +/// the HTML embeds in a static document. An outer/inner div wrapper with +/// a 450px design width applies a uniform CSS-transform scale when the +/// viewport is narrower, so the plot shrinks in proportion instead of +/// squashing. +#[derive(Default, Debug, Clone, Copy)] +pub struct RenderHints { + pub is_notebook: bool, + pub is_positron: bool, + pub output_width_px: Option, +} + +impl RenderHints { + pub fn from_request(header: &MessageHeader, content: &Value) -> Self { + let session = header.session.as_str(); + // Positron's supervisor tags every session it manages with a + // `ggsql-` prefix; standalone Jupyter/Quarto uses UUIDs without one. + let is_positron = session.starts_with("ggsql-"); + let is_notebook = session.contains("notebook"); + let output_width_px = content + .get("positron") + .and_then(|p| p.get("output_width_px")) + .and_then(|v| v.as_u64()) + .and_then(|v| u32::try_from(v).ok()); + Self { + is_notebook, + is_positron, + output_width_px, + } + } +} + /// Format execution result as Jupyter display_data content /// /// Returns `Some(Value)` for results that should be displayed, or `None` for @@ -24,9 +69,9 @@ use serde_json::{json, Value}; /// "transient": { ... } /// } /// ``` -pub fn format_display_data(result: ExecutionResult) -> Option { +pub fn format_display_data(result: ExecutionResult, hints: &RenderHints) -> Option { match result { - ExecutionResult::Visualization { spec } => Some(format_vegalite(spec)), + ExecutionResult::Visualization { spec } => Some(format_vegalite(spec, hints)), ExecutionResult::DataFrame(df) => { // DDL statements return DataFrames with 0 columns - don't display anything if df.width() == 0 { @@ -54,17 +99,28 @@ fn format_connection_changed(display_name: &str) -> Value { } /// Format Vega-Lite visualization as display_data -fn format_vegalite(spec: String) -> Value { - let spec_value: Value = serde_json::from_str(&spec).unwrap_or_else(|e| { +fn format_vegalite(spec: String, hints: &RenderHints) -> Value { + let html = vegalite_html(&spec, hints); + json!({ + "data": { + "text/html": html, + "text/plain": "Vega-Lite visualization".to_string() + }, + "metadata": {}, + "transient": {}, + "output_location": "plot" + }) +} + +/// Generate the HTML wrapper that embeds a Vega-Lite spec via vega-embed. +pub fn vegalite_html(spec: &str, hints: &RenderHints) -> String { + let spec_value: Value = serde_json::from_str(spec).unwrap_or_else(|e| { tracing::error!("Failed to parse Vega-Lite JSON: {}", e); json!({"error": "Invalid Vega-Lite JSON"}) }); - // Generate HTML with embedded Vega-Embed for universal compatibility - // Use require.js approach for Jupyter compatibility let spec_json = serde_json::to_string(&spec_value).unwrap_or_else(|_| "{}".to_string()); - // Generate unique ID for this visualization use std::time::{SystemTime, UNIX_EPOCH}; let timestamp = SystemTime::now() .duration_since(UNIX_EPOCH) @@ -72,7 +128,93 @@ fn format_vegalite(spec: String) -> Value { .as_millis(); let vis_id = format!("vis-{}", timestamp); - let html = format!( + if hints.is_positron { + positron_vegalite_html(&spec_json, &vis_id, hints.is_notebook) + } else { + standalone_vegalite_html(&spec_json, &vis_id) + } +} + +/// Positron template: plain 400px container with no self-installed layout +/// observers. Console sessions additionally upgrade the container to `100vh` +/// when it lives inside `.positron-output-container`, letting Vega-Lite's +/// own container observer keep the Plots pane responsive. Notebook sessions +/// skip that override and keep a stable 400px box. +fn positron_vegalite_html(spec_json: &str, vis_id: &str, is_notebook: bool) -> String { + let pane_override_js = if is_notebook { + "" + } else { + "var container = document.getElementById(visId);\n\ + if (container && container.closest('.positron-output-container')) {\n\ + container.style.height = '100vh';\n\ + }\n" + }; + + format!( + r#"
+ +"#, + vis_id = vis_id, + spec_json = spec_json, + pane_override_js = pane_override_js + ) +} + +/// Standalone template: outer/inner div wrapper driving a uniform +/// scale-to-fit. The inner div holds a 450px design width; when the outer +/// container measures narrower, a CSS transform scales the inner block +/// proportionally and the outer height follows the scaled content. A +/// `ResizeObserver` on the outer div keeps the transform current as the +/// document viewport resizes. +fn standalone_vegalite_html(spec_json: &str, vis_id: &str) -> String { + format!( r#"
@@ -83,10 +225,7 @@ var visId = '{vis_id}'; var minWidth = 450; var inner = document.getElementById(visId); var outer = document.getElementById(visId + '-outer'); -if (inner.closest('.positron-output-container')) {{ -inner.style.height = '100vh'; -}} -var options = {{"actions": true}}; +var options = {{"actions": true, "renderer": "svg"}}; function scaleToFit(o, i) {{ var available = o.clientWidth; if (available < minWidth) {{ @@ -150,21 +289,7 @@ console.error('Failed to load Vega libraries:', err); "#, vis_id = vis_id, spec_json = spec_json - ); - - json!({ - "data": { - // HTML with embedded vega-embed for rendering - "text/html": html, - - // Text fallback - "text/plain": "Vega-Lite visualization".to_string() - }, - "metadata": {}, - "transient": {}, - // Route to Positron Plots pane - "output_location": "plot" - }) + ) } /// Format DataFrame as HTML table @@ -234,7 +359,8 @@ mod tests { fn test_vegalite_format() { let spec = r#"{"mark": "point"}"#.to_string(); let result = ExecutionResult::Visualization { spec }; - let display = format_display_data(result).expect("Visualization should return Some"); + let display = format_display_data(result, &RenderHints::default()) + .expect("Visualization should return Some"); assert!(display["data"]["text/html"].is_string()); assert!(display["data"]["text/plain"].is_string()); @@ -247,7 +373,7 @@ mod tests { // DDL statements return DataFrames with 0 columns let df = DataFrame::new(Vec::::new()).unwrap(); let result = ExecutionResult::DataFrame(df); - let display = format_display_data(result); + let display = format_display_data(result, &RenderHints::default()); assert!( display.is_none(), @@ -262,7 +388,7 @@ mod tests { // SELECT with 0 rows but columns should still display let df = DataFrame::new(vec![Column::new("x".into(), Vec::::new())]).unwrap(); let result = ExecutionResult::DataFrame(df); - let display = format_display_data(result); + let display = format_display_data(result, &RenderHints::default()); assert!( display.is_some(), @@ -277,4 +403,122 @@ mod tests { "<script>alert('xss')</script>" ); } + + fn positron_console() -> RenderHints { + RenderHints { + is_notebook: false, + is_positron: true, + output_width_px: None, + } + } + + fn positron_notebook() -> RenderHints { + RenderHints { + is_notebook: true, + is_positron: true, + output_width_px: Some(589), + } + } + + #[test] + fn test_positron_html_has_no_observer_feedback_loop() { + // Positron templates must not install a `ResizeObserver` or a + // `scaleToFit` transform: Positron animates the output slot during + // reveal, and either one would re-lay out on every animation frame. + for hints in [positron_console(), positron_notebook()] { + let html = vegalite_html(r#"{"mark": "point"}"#, &hints); + assert!( + !html.contains("new ResizeObserver"), + "Positron HTML must not install a ResizeObserver (hints={:?})", + hints + ); + assert!( + !html.contains("scaleToFit"), + "Positron HTML must not include scaleToFit (hints={:?})", + hints + ); + } + } + + #[test] + fn test_console_html_fills_positron_plots_pane() { + let html = vegalite_html(r#"{"mark": "point"}"#, &positron_console()); + assert!( + html.contains(".positron-output-container"), + "HTML must detect Positron's plots pane for responsive height" + ); + assert!( + html.contains("100vh"), + "HTML must scale to 100vh inside the plots pane" + ); + assert!( + html.contains("height: 400px"), + "HTML must set a 400px baseline height for console output" + ); + } + + #[test] + fn test_notebook_html_skips_pane_override() { + let html = vegalite_html(r#"{"mark": "point"}"#, &positron_notebook()); + assert!( + html.contains("height: 400px"), + "notebook container uses the shared 400px baseline" + ); + assert!( + !html.contains(".positron-output-container"), + "notebook HTML must not carry the plots-pane 100vh override" + ); + assert!( + !html.contains("100vh"), + "notebook HTML must not reach for 100vh" + ); + } + + #[test] + fn test_standalone_html_uses_scale_to_fit() { + // Standalone (Jupyter/Quarto) renders into a static document and + // wraps the plot in the outer/inner div + min-width scale-to-fit so + // narrow viewports shrink the plot proportionally. + let html = vegalite_html(r#"{"mark": "point"}"#, &RenderHints::default()); + assert!( + html.contains("min-width: 450px"), + "standalone HTML must use the 450px design width" + ); + assert!( + html.contains("scaleToFit"), + "standalone HTML must uniformly scale narrow viewports" + ); + assert!( + html.contains("new ResizeObserver"), + "standalone HTML must observe container resizes" + ); + assert!( + html.contains("-outer"), + "standalone HTML must wrap the inner div in an overflow-hidden outer div" + ); + assert!( + !html.contains(".positron-output-container"), + "standalone HTML must not carry the Positron plots-pane branch" + ); + } + + #[test] + fn test_from_request_detects_positron_sessions() { + let header = |session: &str| MessageHeader { + msg_id: String::new(), + session: session.to_string(), + username: String::new(), + date: String::new(), + msg_type: String::new(), + version: String::new(), + }; + let console = RenderHints::from_request(&header("ggsql-c2a5a97b"), &json!({})); + assert!(console.is_positron && !console.is_notebook); + + let notebook = RenderHints::from_request(&header("ggsql-notebook-abc"), &json!({})); + assert!(notebook.is_positron && notebook.is_notebook); + + let standalone = RenderHints::from_request(&header("abcd-efgh-1234"), &json!({})); + assert!(!standalone.is_positron && !standalone.is_notebook); + } } diff --git a/ggsql-jupyter/src/kernel.rs b/ggsql-jupyter/src/kernel.rs index 34962ff4..b9844c60 100644 --- a/ggsql-jupyter/src/kernel.rs +++ b/ggsql-jupyter/src/kernel.rs @@ -5,7 +5,7 @@ use crate::connection; use crate::data_explorer::{DataExplorerState, RpcResponse}; -use crate::display::format_display_data; +use crate::display::{format_display_data, RenderHints}; use crate::executor::{self, ExecutionResult, QueryExecutor}; use crate::message::{ConnectionInfo, JupyterMessage, MessageHeader}; use anyhow::Result; @@ -291,8 +291,15 @@ impl KernelServer { let content = &parent.content; let code = content["code"].as_str().unwrap_or(""); let silent = content["silent"].as_bool().unwrap_or(false); + let hints = RenderHints::from_request(&parent.header, content); - tracing::info!("Executing code ({} chars, silent={})", code.len(), silent); + tracing::info!( + "Executing code ({} chars, silent={}, notebook={}, width_px={:?})", + code.len(), + silent, + hints.is_notebook, + hints.output_width_px + ); // Increment execution counter if !silent { @@ -331,7 +338,7 @@ impl KernelServer { // Per Jupyter spec: execute_result includes execution_count // Only send if there's something to display (DDL returns None) if !silent && !is_connection_changed { - if let Some(display_data) = format_display_data(exec_result) { + if let Some(display_data) = format_display_data(exec_result, &hints) { // Build message content, including output_location if present let mut content = json!({ "execution_count": self.execution_count, From ece959f7bd94d8b20bc45ce8a36223d47119d388 Mon Sep 17 00:00:00 2001 From: Thomas Lin Pedersen Date: Thu, 23 Apr 2026 17:21:18 +0200 Subject: [PATCH 2/3] update changelog --- src/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/CHANGELOG.md b/src/CHANGELOG.md index 57d3aa17..2fd15e31 100644 --- a/src/CHANGELOG.md +++ b/src/CHANGELOG.md @@ -4,6 +4,12 @@ - ODBC is now turned on for the CLI as well (#344) +### Fixed + +- Rendering of inline plots in Positron had a bad interaction with how we +handled auto-resizing in the plot pane. We now have a per-output-location path +in the Jupyter kernel (#360) + ### Removed - Removed polars from dependency list along with all its transient dependencies. Rewrote DataFrame struct on top of arrow (#350) From b2355bd3554b00d85c8aa24af66a8573cc863537 Mon Sep 17 00:00:00 2001 From: Thomas Lin Pedersen Date: Thu, 23 Apr 2026 17:21:47 +0200 Subject: [PATCH 3/3] move changelog to root --- src/CHANGELOG.md => CHANGELOG.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/CHANGELOG.md => CHANGELOG.md (100%) diff --git a/src/CHANGELOG.md b/CHANGELOG.md similarity index 100% rename from src/CHANGELOG.md rename to CHANGELOG.md