fix: address Copilot review findings

- Bump declared Rust MSRV to 1.88: the native-writer dependency graph (parley 0.9) requires it, so 1.86 was not actually supported.

- Validate dpi in SVGWriter/PDFWriter constructors (rejects non-positive and non-finite values) instead of letting them reach the renderer.

- Add Python integration tests for builtin datasets (ggsql:penguins, ggsql:airquality) and dpi validation.

Author: cpsievert

CHANGELOG.md

@@ -12,6 +12,7 @@
 
 - Upgraded to ggsql Rust crate v0.5.2 (from v0.3.3). See the [crate changelog](https://github.com/posit-dev/ggsql/blob/main/CHANGELOG.md) for the many language and rendering improvements in 0.4.0–0.5.2.
 - Upgraded the Arrow dependency from v56 to v58.
+- Declared Rust MSRV is now 1.88 (required by the native-writer dependency graph, e.g. parley 0.9).
 - Temporarily vendored the ggsql crate (via `[patch.crates-io]`) with its duckdb pin relaxed to `~1.10506`, fixing bundled duckdb builds that fail on Windows/MSVC ([duckdb/duckdb-rs#783](https://github.com/duckdb/duckdb-rs/issues/783)). This will be removed once a ggsql release depends on duckdb `~1.10506`.
 - `VegaLiteWriter.render_chart()` and `render_altair()` now accept `validate`, `height`, and `width` as explicit keyword arguments. `height` and `width` default to `None`, which preserves whatever dimensions ggsql produces. `validate` defaults to `False` (same behavior as before, now a named parameter).
 

Cargo.toml

@@ -7,7 +7,7 @@ license = "MIT"
 repository = "https://github.com/posit-dev/ggsql-python"
 homepage = "https://ggsql.org"
 description = "Python bindings for ggsql"
-rust-version = "1.86"
+rust-version = "1.88"
 
 [lib]
 name = "_ggsql"

python/ggsql/__init__.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import json
+import math
 import warnings
 from pathlib import Path
 from typing import Any, Union
@@ -101,6 +102,12 @@ def _json_to_altair_chart(vegalite_json: str, **kwargs: Any) -> AltairChart:
     return altair.Chart.from_json(vegalite_json, **kwargs)
 
 
+def _validate_dpi(dpi: float) -> None:
+    """Reject non-positive or non-finite DPI before reaching the renderer."""
+    if not math.isfinite(dpi) or dpi <= 0:
+        raise ValueError(f"dpi must be a positive, finite number, got {dpi!r}")
+
+
 class SVGWriter:
     """SVG output writer using ggsql's native renderer.
 
@@ -120,6 +127,7 @@ class SVGWriter:
     """
 
     def __init__(self, width: int = 600, height: int = 400, dpi: float = 96.0) -> None:
+        _validate_dpi(dpi)
         self._inner = _RustSVGWriter(width, height, dpi)
 
     def render(self, spec: Spec) -> str:
@@ -152,6 +160,7 @@ class PDFWriter:
     """
 
     def __init__(self, width: int = 600, height: int = 400, dpi: float = 96.0) -> None:
+        _validate_dpi(dpi)
         self._inner = _RustPDFWriter(width, height, dpi)
 
     def render(self, spec: Spec) -> bytes:

tests/test_ggsql.py

@@ -794,3 +794,39 @@ def test_save_with_dimensions(self, spec, tmp_path):
     def test_save_unknown_extension_raises(self, spec, tmp_path):
         with pytest.raises(ValueError, match="Unsupported file extension"):
             ggsql.save(spec, tmp_path / "plot.png")
+
+    def test_save_invalid_dpi_raises(self, spec, tmp_path):
+        with pytest.raises(ValueError, match="dpi must be a positive"):
+            ggsql.save(spec, tmp_path / "plot.svg", dpi=0)
+
+
+class TestWriterDpiValidation:
+    @pytest.mark.parametrize("bad_dpi", [0, -1, -96.0, float("nan"), float("inf")])
+    def test_svg_writer_rejects_invalid_dpi(self, bad_dpi):
+        with pytest.raises(ValueError, match="dpi must be a positive"):
+            ggsql.SVGWriter(dpi=bad_dpi)
+
+    @pytest.mark.parametrize("bad_dpi", [0, -1, -96.0, float("nan"), float("inf")])
+    def test_pdf_writer_rejects_invalid_dpi(self, bad_dpi):
+        with pytest.raises(ValueError, match="dpi must be a positive"):
+            ggsql.PDFWriter(dpi=bad_dpi)
+
+
+class TestBuiltinData:
+    """Integration coverage for builtin datasets (ggsql:... relations)."""
+
+    def test_execute_builtin_dataset(self):
+        reader = ggsql.DuckDBReader("duckdb://memory")
+        spec = reader.execute(
+            "SELECT * FROM ggsql:penguins "
+            "VISUALISE DRAW point MAPPING bill_len AS x, bill_dep AS y"
+        )
+        assert spec is not None
+
+    def test_builtin_dataset_renders_svg(self):
+        reader = ggsql.DuckDBReader("duckdb://memory")
+        spec = reader.execute(
+            "VISUALISE FROM ggsql:airquality DRAW point MAPPING Temp AS x, Ozone AS y"
+        )
+        svg = ggsql.SVGWriter().render(spec)
+        assert "<svg" in svg