diff --git a/CHANGELOG.md b/CHANGELOG.md index f8d10e3d..791c4f5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,12 +63,17 @@ in the README). integers, plus finite floating arrays, including non-native-endian NumPy arrays. Float16/32 values widen exactly to the f64 token contract. It preserves the existing 64-bit identities and duplicate-row errors; mixed - objects, NUL-containing Python sequences, dates, and non-finite row - diagnostics stay on the conservative Python reference path. Highly padded - Python string/bytes sequences also stay there to bound fixed-width temporary - memory. Fixed-width NumPy strings retain their exact embedded-NUL semantics - natively. - This adds the C ABI v43 transition-key kernel. + objects, dates, and non-finite row diagnostics stay on the conservative + Python reference path. Highly padded Python string/bytes sequences also stay + there to bound fixed-width temporary memory. Fixed-width NumPy strings retain + their exact embedded-NUL semantics natively. + Routing follows the values rather than the container, so a key column passed + as a pandas Series or as homogeneous object storage — what `data=df, + key="id"` actually resolves to — takes the same path as an ndarray instead of + falling back. Only keys that *end* in NUL stay on the reference encoder, + where fixed-width padding would otherwise absorb them; interior NULs are + encoded natively. + This adds the C ABI v44 transition-key kernel. - Host theme changes made through an ancestor `data-theme` attribute now refresh canvas/SVG paint just like `.dark` class and inline-style changes; DOM chrome continues to follow the cascade automatically. diff --git a/benchmarks/test_codspeed_animation.py b/benchmarks/test_codspeed_animation.py index e64ea225..cffb07d1 100644 --- a/benchmarks/test_codspeed_animation.py +++ b/benchmarks/test_codspeed_animation.py @@ -19,7 +19,7 @@ import xy from xy import kernels as k -from xy.components import _encode_transition_keys +from xy.components import _encode_transition_keys, _fixed_transition_key_values N = 100_000 @@ -72,6 +72,15 @@ def payload_figures(animation_data): def test_animation_encode_100k_stable_keys(benchmark, animation_data) -> None: """Track the public list-to-native path, including homogeneous routing.""" _x, _y, keys = animation_data + # Routing is the whole point of this row, and nothing about the result + # distinguishes the two paths — both return the same F-order planes. Prove + # the native kernel accepts this input before timing anything, so a + # layout-contract drift shows up as a red row instead of a silent ~7× + # regression into the reference encoder. + fixed = _fixed_transition_key_values(keys) + assert fixed is not None, "benchmark keys must route to the native encoder" + assert k.transition_keys_fixed(fixed, "benchmark key") is not None + encoded = benchmark(_encode_transition_keys, keys, N, "benchmark key") assert encoded.shape == (N, 2) assert encoded.dtype == np.uint32 diff --git a/python/xy/_native.py b/python/xy/_native.py index d7b3153b..9f514213 100644 --- a/python/xy/_native.py +++ b/python/xy/_native.py @@ -24,7 +24,7 @@ from .config import MAX_CONTOUR_WORK, MAX_SCREEN_DIM -ABI_VERSION = 43 +ABI_VERSION = 44 # Rust reports invalid arguments (and, via the ffi_guard panic shield, any # internal panic) by returning `usize::MAX` from size-returning entry points. @@ -971,11 +971,17 @@ def transition_keys_fixed( """Encode homogeneous fixed-width transition keys in one native row scan. The returned ``(N, 2)`` array is Fortran-contiguous: its two ``u32`` - columns are the caller-allocated lo/hi planes written by Rust and can be - shipped independently without another full-size copy. ``None`` asks the - policy layer to use its scalar oracle for an invalid value (for example a - non-finite float or invalid Unicode scalar), preserving its precise - user-facing error. + columns are the caller-allocated lo/hi planes written by Rust, so an + unreordered result ships each plane without another full-size copy. + Reordering the rows (the line-like geometry sort, or a finite-row + selection) hands back C-order and restores the usual per-column copy at + ship time. + + ``None`` asks the policy layer to use its scalar oracle for a value this + kernel declines (a non-finite float, an invalid Unicode scalar), + preserving its precise user-facing error. An invalid *argument* is a bug + here rather than a data property, and raises instead of degrading + silently into the reference path. """ records = np.asarray(values) if records.ndim != 1 or records.dtype.hasobject: @@ -1040,6 +1046,11 @@ def transition_keys_fixed( ) if status == 3: raise ValueError(f"{label} produced an identity digest collision") + if status == 4: + raise RuntimeError( + "native transition-key encoder rejected the " + f"{records.dtype.str!r} layout it was handed (kind {kind}, width {width})" + ) raise RuntimeError(f"native transition-key encoder returned unknown status {status}") diff --git a/python/xy/components.py b/python/xy/components.py index 98602177..07e02102 100644 --- a/python/xy/components.py +++ b/python/xy/components.py @@ -3872,62 +3872,101 @@ def _transition_key_token(value: Any, index: int) -> bytes: _TRANSITION_KEY_SEQUENCE_MAX_PADDING_RATIO = 8 -def _fixed_transition_key_values(value: Any) -> np.ndarray | None: - """Return a conservative homogeneous array for the native key encoder.""" - if type(value) is np.ndarray: - arr = value - elif isinstance(value, (list, tuple)) and value: - first = value[0].item() if isinstance(value[0], np.generic) else value[0] - item_type = type(first) - if item_type not in (str, bytes, bool, int, float): +_TRANSITION_KEY_SEQUENCE_KINDS = { + str: {"U"}, + bytes: {"S"}, + bool: {"b"}, + int: {"i", "u"}, + float: {"f"}, +} + + +def _column_ndarray(value: Any) -> np.ndarray | None: + """Unwrap a dtype-carrying column (pandas/polars Series) as plain NumPy. + + `data=df, key="id"` resolves to a Series, not an ndarray, so without this + the whole native path would be unreachable from the documented idiom. + """ + if isinstance(value, (str, bytes)) or not hasattr(value, "to_numpy"): + return None + try: + arr = value.to_numpy() + except (TypeError, ValueError): + # Extension arrays may refuse a zero-copy conversion; the reference + # encoder reads the same values through the object protocol anyway. + return None + return arr if type(arr) is np.ndarray else None + + +def _fixed_sequence_array(sequence: Any) -> np.ndarray | None: + """Convert a homogeneous scalar sequence to fixed-width NumPy storage. + + Returns None whenever the conversion would change an identity or cost + disproportionate memory, leaving the row to the Python reference encoder. + """ + items = sequence.tolist() if isinstance(sequence, np.ndarray) else sequence + if not items: + return None + first = items[0] + if isinstance(first, np.generic): + first = first.item() + item_type = type(first) + if item_type not in _TRANSITION_KEY_SEQUENCE_KINDS: + return None + if set(map(type, items)) != {item_type}: + # Either genuinely mixed, or NumPy scalars needing ``.item()``. Retry + # through the scalar protocol, then insist on one exact builtin type: + # never coerce mixed bool/int or int/float keys, whose scalar tokens + # are deliberately type-sensitive. + items = [raw.item() if isinstance(raw, np.generic) else raw for raw in items] + if set(map(type, items)) != {item_type}: return None - total_units = 0 - max_units = 0 - for raw in value: - item = raw.item() if isinstance(raw, np.generic) else raw - if type(item) is not item_type: - # In particular, never coerce mixed bool/int or int/float - # keys: their scalar tokens are deliberately type-sensitive. - return None - if (item_type is str and "\x00" in item) or (item_type is bytes and b"\x00" in item): - # Fixed-width storage cannot distinguish padding from an - # explicitly trailing NUL supplied in a Python scalar. - return None - if item_type in (str, bytes): - units = len(item) - total_units += units - max_units = max(max_units, units) - if item_type in (str, bytes): - unit_bytes = 4 if item_type is str else 1 - # NumPy's fixed-width sequence conversion costs N × max width. - # Keep a single long outlier from turning a compact Python list - # into a huge temporary before Rust can scan it. - padded_bytes = len(value) * max(max_units, 1) * unit_bytes - source_bytes = max(total_units * unit_bytes, 1) - if ( - padded_bytes > _TRANSITION_KEY_SEQUENCE_MAX_FIXED_BYTES - or padded_bytes > source_bytes * _TRANSITION_KEY_SEQUENCE_MAX_PADDING_RATIO - ): - return None - try: - arr = np.asarray(value) - except (OverflowError, TypeError, ValueError): + if item_type in (str, bytes): + lengths = list(map(len, items)) + unit_bytes = 4 if item_type is str else 1 + # NumPy's fixed-width conversion costs N × longest key. Keep a single + # long outlier from turning a compact sequence into a huge temporary + # before Rust can scan it. The ratio compares padding to payload in + # those same fixed-width units — it is not a bound on the growth over + # the source objects' own footprint; the absolute cap is. + padded_bytes = len(items) * max(max(lengths), 1) * unit_bytes + source_bytes = max(sum(lengths) * unit_bytes, 1) + if ( + padded_bytes > _TRANSITION_KEY_SEQUENCE_MAX_FIXED_BYTES + or padded_bytes > source_bytes * _TRANSITION_KEY_SEQUENCE_MAX_PADDING_RATIO + ): return None - expected_kinds = { - str: {"U"}, - bytes: {"S"}, - bool: {"b"}, - int: {"i", "u"}, - float: {"f"}, - } - if arr.dtype.kind not in expected_kinds[item_type]: - # NumPy may coerce an integer list outside its fixed-width range - # to f64. That would change the type-sensitive ``i:`` token into - # an ``f:`` token, so leave arbitrary-width integers to Python. + # A key that *ends* in NUL is indistinguishable from padding once + # stored fixed-width, which would silently retokenize it. Interior + # NULs survive the round trip and stay on the native path. + nul = "\x00" if item_type is str else b"\x00" + if any(item.endswith(nul) for item in items): return None - else: + try: + arr = np.asarray(items) + except (OverflowError, TypeError, ValueError): return None - if arr.ndim != 1: + if arr.dtype.kind not in _TRANSITION_KEY_SEQUENCE_KINDS[item_type]: + # NumPy may coerce an integer list outside its fixed-width range to + # f64. That would change the type-sensitive ``i:`` token into an + # ``f:`` token, so leave arbitrary-width integers to Python. + return None + return arr + + +def _fixed_transition_key_values(value: Any) -> np.ndarray | None: + """Return a conservative homogeneous array for the native key encoder.""" + arr = value if type(value) is np.ndarray else _column_ndarray(value) + if arr is None: + if not isinstance(value, (list, tuple)): + return None + arr = _fixed_sequence_array(value) + elif arr.dtype.hasobject: + # Object storage — a pandas string column, or an explicitly + # object-dtype array — still reaches the encoder when every row + # carries the same builtin scalar type. + arr = _fixed_sequence_array(arr) if arr.ndim == 1 else None + if arr is None or arr.ndim != 1: return None if arr.dtype.kind == "U" and arr.dtype.itemsize > 0 and arr.dtype.itemsize % 4 == 0: return arr diff --git a/scripts/abi_smoke.py b/scripts/abi_smoke.py index 22863c2d..e8b42103 100644 --- a/scripts/abi_smoke.py +++ b/scripts/abi_smoke.py @@ -568,8 +568,44 @@ def ok(cond: bool, msg: str) -> None: ctypes.byref(transition_first), ctypes.byref(transition_index), ) - == 1, - "transition_keys_fixed non-empty/null rejected", + == 4, + "transition_keys_fixed non-empty/null is an argument error", + ) + nonfinite_records = array("d", [float("inf")]) + nonfinite_low = array("I", [99]) + nonfinite_high = array("I", [99]) + transition_first = ctypes.c_size_t(size_max) + transition_index = ctypes.c_size_t(size_max) + ok( + lib.xy_transition_keys_fixed( + _ptr(nonfinite_records, ctypes.c_uint8), + 1, + 8, + 5, + 0, + _ptr(nonfinite_low, ctypes.c_uint32), + _ptr(nonfinite_high, ctypes.c_uint32), + ctypes.byref(transition_first), + ctypes.byref(transition_index), + ) + == 1 + and transition_first.value == 0, + "transition_keys_fixed declines non-finite data with its row", + ) + ok( + lib.xy_transition_keys_fixed( + _ptr(nonfinite_records, ctypes.c_uint8), + 1, + 3, + 0, + 0, + _ptr(nonfinite_low, ctypes.c_uint32), + _ptr(nonfinite_high, ctypes.c_uint32), + ctypes.byref(transition_first), + ctypes.byref(transition_index), + ) + == 4, + "transition_keys_fixed bad layout is an argument error", ) small_unique = array("I", [99] * 2) ok( diff --git a/spec/design/animation.md b/spec/design/animation.md index de758613..a4569a0a 100644 --- a/spec/design/animation.md +++ b/spec/design/animation.md @@ -66,17 +66,31 @@ signed or unsigned integers, and finite floating arrays take one bulk native path, including non-native endian NumPy arrays; f16/f32 values widen exactly to the f64 token contract. Its identities and first-duplicate row diagnostics are byte-for-byte compatible with the original Python encoder. -Python remains the conservative reference path for mixed objects, -NUL-containing Python string/bytes sequences, dates, and datetimes, and -supplies the exact row error for non-finite numbers. Python sequences whose -fixed-width conversion would exceed the byte budget or 8× padding ratio also -stay on that path, bounding temporary memory under skewed key lengths. -Fixed-width NumPy `U`/`S` records keep their exact trailing-padding and -embedded-NUL semantics in the native path. This keeps the complete public +Routing is over values, not containers: lists, tuples, NumPy arrays, and +`to_numpy`-carrying columns (the pandas Series a `data=df, key="id"` lookup +actually returns) all reach it, and object storage qualifies whenever every row +holds the same builtin scalar type — otherwise the documented DataFrame idiom +would be the one shape that never took the fast path. +Python remains the conservative reference path for mixed objects, dates, +datetimes, and arbitrary-width integers, and supplies the exact row error for +non-finite and missing values. Sequences whose fixed-width conversion would +exceed the byte budget or 8× padding ratio also stay on that path, bounding +temporary memory under skewed key lengths. +Fixed-width `U`/`S` records keep their exact trailing-padding and embedded-NUL +semantics in the native path; a Python key that *ends* in NUL would be +indistinguishable from that padding, so only those stay on the reference path — +interior NULs survive the round trip. This keeps the complete public grammar—strings, finite numbers, booleans, bytes, dates, datetimes, and NumPy equivalents—without pushing Python object policy through the C ABI. Missing, unsupported, wrong-length, or duplicate values fail during figure construction. -Line-like keys follow the same stable geometry sort as their coordinates. +The kernel separates "declined this data, use the oracle" from "this layout is +not in the ABI": only the first falls back, so a contract drift between the +ctypes gate and the Rust layout check raises instead of silently costing the +whole speedup. +Line-like keys follow the same stable geometry sort as their coordinates; the +encoder hands back Fortran-order planes that ship without a per-column copy, +but that sort and any finite-row selection reorder through NumPy advanced +indexing, which returns C-order and restores the copy at ship time. Errorbar point keys are role-qualified after expansion so the main segment and caps remain unique and stable. diff --git a/spec/design/rust-engine.md b/spec/design/rust-engine.md index 673bb1da..cb4a2c30 100644 --- a/spec/design/rust-engine.md +++ b/spec/design/rust-engine.md @@ -2,7 +2,7 @@ **Status:** design. Decides what lives in Rust vs Python and how the FFI seam evolves without rewrites. Grounded in the shipped engine: single-dependency -cdylib (`src/lib.rs`, ABI v43; `png` is the one crate, for static export), +cdylib (`src/lib.rs`, ABI v44; `png` is the one crate, for static export), ctypes binding (`_native.py`), and dispatch in `kernels.py`. The native core is required — `kernels.py` raises a clear ImportError when it can't load, with no pure-Python fallback. @@ -27,7 +27,7 @@ clear ImportError when it can't load, with no pure-Python fallback. |---|---|---| | zone maps, encode_f32, m4, bin_2d, bin_2d_mean_color, min_max, histogram_uniform, normalize_f32, range/validity indices, polygon (lasso) selection, local_log_density | Rust (ABI v41) | correct — new equal-length x/y columns use a paired zone-map call with bit-identical per-column reductions; lasso ray casting (`xy_polygon_select`, §34) walks edges inside the point loop instead of one NumPy pass per edge, and buckets edges by y so a point tests only those spanning its row — the answer is the same crossing parity, without the per-edge full-length temporaries that made a 2048-vertex lasso cost ~370 ms over 160k candidates. Bucketing is declined below 16 vertices (the build outweighs the scans it saves) and for a non-finite polygon (slab bounds stop meaning anything); above 16 the slab count tracks the vertex count, capped at 256 and shed further so the CSR never exceeds 2²⁰ entries, which bounds both the allocation and its u32 cursors independently of what a caller passes; full-domain density first paint fuses binning with uniform or counted-u8 overlay sampling while retaining exact standalone outputs; mean-color binning (LOD doc §2) is an integer-only pipeline (checked-in sRGB⇄linear-u16 tables, alpha-weighted u64 sums) so grids are bitwise deterministic across thread counts and platforms; mesh/rectangle validity scans consume only columns not already proven finite by zone metadata | | fixed-width string/bytes/bool factorization | Rust (ABI v36) | correct — compact palettes use a bounded L1-resident codebook with full-record collision checks and emit exact counts; U1 uses a direct Unicode-scalar table with endian support; ≥512k rows probe a prefix then encode disjoint chunks in parallel, merging late labels by canonical first-row order before any retry; Python sees only unique labels and retains display-label ordering policy | -| stable animation-key encoding for homogeneous fixed-width string/bytes/bool/integer/float columns | Rust (ABI v43) | correct — one borrowed row scan emits the two caller-owned u32 identity planes and reports the first duplicate pair; f16/f32 widen exactly to the f64 token contract and non-native arrays carry an explicit endian flag; Python validates shape/length, bounds padded temporaries for skewed string/bytes sequences, assembles exact public errors, and retains the reference encoder for mixed objects, NUL-containing Python sequences, dates, and non-finite row diagnostics | +| stable animation-key encoding for homogeneous fixed-width string/bytes/bool/integer/float columns | Rust (ABI v43) | correct — one borrowed row scan emits the two caller-owned u32 identity planes and reports the first duplicate pair; f16/f32 widen exactly to the f64 token contract and non-native arrays carry an explicit endian flag; Python validates shape/length, unwraps `to_numpy` columns and homogeneous object storage so a DataFrame key column routes at all, bounds padded temporaries for skewed string/bytes sequences, assembles exact public errors, and retains the reference encoder for mixed objects, trailing-NUL Python sequences, dates, and non-finite row diagnostics. Declined *data* (status 1) falls back; an out-of-contract *layout* (status 4) raises, so the two cannot be confused into a silent slow path | | static display-list raster, row-banded polyline/point/segment paint, batched fill+stroke triangle meshes, affine scatter projection plus typed color/size resolution, density/heatmap colormap and sampling | Rust (ABI v36) | correct — commands borrow f32/u8 payload or canonical spans synchronously; compact stratified sampling reuses factorization counts; batched/banded output is byte-identical | | signal processing: `xy_rfft`, `xy_welch_spectra`, `xy_spectrogram` | Rust (ABI v36) | correct — O(N) transforms over sample columns; window/segment policy stays in Python | | geometry/triangulation: `xy_delaunay_triangles`, `xy_polygon_triangles`, `xy_marching_squares`, `xy_marching_triangles`, `xy_streamlines`, `xy_vector_segments`, `xy_quad_mesh_triangles`, `xy_sector_triangles`, `xy_indexed_triangles`, `xy_triangle_edges` | Rust (ABI v36) | correct — output is screen-bounded index/vertex buffers; level choice and styling stay in Python | diff --git a/spec/process/perf-audit-2026-07-22.md b/spec/process/perf-audit-2026-07-22.md index f73ec2d0..78f09941 100644 --- a/spec/process/perf-audit-2026-07-22.md +++ b/spec/process/perf-audit-2026-07-22.md @@ -128,18 +128,28 @@ there is O(sample), not O(N). ### Medium -3. **Resolved after this audit — animation `key=` encoding hashed homogeneous - columns row-by-row in Python** (`components.py`). ABI v43 adds a borrowed - fixed-width native encoder for homogeneous string, bytes, bool, integer, - and finite floating keys (f16/f32 widen exactly to f64). It emits both u32 - identity planes in one pass and reports the first duplicate pair so Python - preserves the exact error - contract. Conservative Python fallback retains mixed-object, - NUL-containing Python sequence, date, and non-finite-row diagnostic +3. **Mostly resolved after this audit — animation `key=` encoding hashed + homogeneous columns row-by-row in Python** (`components.py`). ABI v43 added + a borrowed fixed-width native encoder for homogeneous string, bytes, bool, + integer, and finite floating keys (f16/f32 widen exactly to f64). It emits + both u32 identity planes in one pass and reports the first duplicate pair so + Python preserves the exact error contract. Conservative Python fallback + retains mixed-object, trailing-NUL, date, and non-finite-row diagnostic semantics; fixed-width NumPy `U`/`S` records keep exact embedded-NUL - semantics natively. + semantics natively. A follow-up extended routing to `to_numpy` columns and + homogeneous object storage — without it the documented `data=df, key="id"` + idiom, which resolves to a Series, never reached the kernel at all. `benchmarks/test_codspeed_animation.py::test_animation_encode_100k_stable_keys` - keeps the original 100k end-to-end row as the regression gate. + keeps the original 100k end-to-end row as the regression gate, and now + asserts the input actually routes so the gate cannot pass on the slow path. + + Residual: the *discarded-work* half of this finding is still open. Keys are + encoded for every row before `_transition_entry` decides, and above + `MAX_ANIMATION_MATCH_ROWS` (200k) it drops them for `snap:key-limit`. A + 1M-row keyed mark still spends ~144 ms (was ~800 ms) producing identities + nothing reads. Fix remains: short-circuit when the fallback is certain, + minding that duplicate-key errors are part of the construction contract and + must still fire. 4. **Direct-tier traces retain f32 wire buffers for the widget lifetime** (`widget.py:55`, `_payload.py:154`). A near-ceiling direct scatter (500k–2M pts) holds f64 canonical + f32 wire simultaneously (~24 B/pt) diff --git a/src/lib.rs b/src/lib.rs index 5f2f0b6b..d6ecc734 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,7 +83,7 @@ unsafe fn borrowed_byte_spans<'a>( /// ABI version — bumped on any signature change. The Python wrapper checks this /// at load time and refuses a mismatched library loudly (§33 comm-versioning /// rule, applied to the in-process boundary). -pub const ABI_VERSION: u32 = 43; +pub const ABI_VERSION: u32 = 44; const FACTORIZE_CAPACITY_EXCEEDED: usize = usize::MAX - 1; #[no_mangle] @@ -97,11 +97,13 @@ pub extern "C" fn xy_abi_version() -> u32 { /// 1/2/4/8 bytes; Unicode width is a positive multiple of four. `swap_endian` /// must be zero or one. /// -/// Returns 0 on success, 1 for invalid arguments or scalar data, 2 for a -/// duplicate token, and 3 for a digest collision. For status 2 or 3, -/// `out_error_first` and `out_error_index` receive the prior/current row -/// indices. Invalid scalar data also records its row in both outputs when they -/// are non-null; invalid pointer/dimension calls do not write error outputs. +/// Returns 0 on success, 1 for scalar data this kernel declines to tokenize +/// (the caller falls back to its reference encoder), 2 for a duplicate token, +/// 3 for a digest collision, and 4 for invalid arguments. Statuses 1, 2, and 3 +/// write `out_error_first`/`out_error_index`: the offending row for 1, and the +/// prior/current pair for 2 and 3. Status 4 writes neither, and is a caller +/// bug rather than a data property — keeping it distinct from 1 stops a +/// layout-contract drift from degrading silently into the slow path. /// /// # Safety /// For non-empty input, `data` addresses `len * width` readable bytes and each @@ -120,34 +122,35 @@ pub unsafe extern "C" fn xy_transition_keys_fixed( out_error_index: *mut usize, ) -> i32 { if !matches!(swap_endian, 0 | 1) { - return 1; + return 4; } if len == 0 { return 0; } if out_error_first.is_null() || out_error_index.is_null() { - return 1; + return 4; } let byte_len = match len.checked_mul(width) { Some(value) if width > 0 => value, - _ => return 1, + _ => return 4, }; if data.is_null() || out_lo.is_null() || out_hi.is_null() { - return 1; + return 4; } let data = std::slice::from_raw_parts(data, byte_len); let low = std::slice::from_raw_parts_mut(out_lo, len); let high = std::slice::from_raw_parts_mut(out_hi, len); - ffi_guard(1, || { + ffi_guard(4, || { match transition::encode_fixed_into(data, width, kind, swap_endian != 0, low, high) { Ok(()) => 0, - Err(transition::TransitionKeyError::Invalid { index }) => { - if let Some(index) = index { + Err(transition::TransitionKeyError::Invalid { index }) => match index { + Some(index) => { *out_error_first = index; *out_error_index = index; + 1 } - 1 - } + None => 4, + }, Err(transition::TransitionKeyError::Duplicate { first, index }) => { *out_error_first = first; *out_error_index = index; @@ -3279,7 +3282,43 @@ mod tests { std::ptr::null_mut(), std::ptr::null_mut(), ), - 1 + 4 + ); + // A layout the caller should never send is status 4, not the + // status-1 "declined this data" that means "use the oracle". + let row = [0u8; 3]; + let mut low = [0u32]; + let mut high = [0u32]; + let mut first = usize::MAX; + let mut index = usize::MAX; + assert_eq!( + xy_transition_keys_fixed( + row.as_ptr(), + 1, + 3, + transition::KIND_UNICODE, + 0, + low.as_mut_ptr(), + high.as_mut_ptr(), + &mut first, + &mut index, + ), + 4 + ); + assert_eq!((first, index), (usize::MAX, usize::MAX)); + assert_eq!( + xy_transition_keys_fixed( + row.as_ptr(), + 1, + 1, + transition::KIND_BYTES, + 7, + low.as_mut_ptr(), + high.as_mut_ptr(), + &mut first, + &mut index, + ), + 4 ); } } diff --git a/tests/test_animation.py b/tests/test_animation.py index 8484b098..c37689d7 100644 --- a/tests/test_animation.py +++ b/tests/test_animation.py @@ -278,6 +278,15 @@ def test_stable_keys_are_type_sensitive_and_deterministic() -> None: ), id="numpy-float64-swapped-subnormal", ), + # Object storage still routes when every row is one builtin type — + # this is the shape a pandas string column arrives in. + pytest.param(np.array(["a", "b"], dtype=object), id="object-array"), + pytest.param(np.array([1.25, 2.5], dtype=object), id="object-floats"), + pytest.param(np.array([3, -4], dtype=object), id="object-ints"), + # An interior NUL survives fixed-width storage intact; only a + # *trailing* one is ambiguous against padding. + pytest.param(["a\x00b", "plain"], id="list-unicode-interior-nul"), + pytest.param([b"a\x00b", b"plain"], id="list-bytes-interior-nul"), ], ) def test_native_transition_key_fast_paths_match_python_reference(keys, monkeypatch) -> None: @@ -304,10 +313,11 @@ def tracked(values: np.ndarray, label: str): "keys", [ pytest.param([1, "1", True, b"1"], id="mixed-builtins"), - pytest.param(np.array(["a", "b"], dtype=object), id="object-array"), - pytest.param(np.array([1.25, 2.5], dtype=object), id="object-floats"), - pytest.param(["a\x00b", "plain"], id="list-unicode-nul"), - pytest.param([b"a\x00b", b"plain"], id="list-bytes-nul"), + pytest.param(np.array([1, "1"], dtype=object), id="object-array-mixed"), + # A trailing NUL is indistinguishable from fixed-width padding, so + # these must keep their exact Python tokens. + pytest.param(["a\x00", "plain"], id="list-unicode-trailing-nul"), + pytest.param([b"a\x00", b"plain"], id="list-bytes-trailing-nul"), pytest.param( [dt.date(2024, 1, 1), dt.date(2024, 1, 2)], id="dates", @@ -389,6 +399,67 @@ def test_transition_key_empty_shape_and_length_semantics() -> None: component_api._encode_transition_keys(["a", "b"], 3, "length key") +@pytest.mark.parametrize( + "column", + [ + pytest.param(["alpha", "beta", "gamma"], id="string-column"), + pytest.param([3, -4, 5], id="integer-column"), + pytest.param([1.5, -0.0, 2.25], id="float-column"), + pytest.param([True, False], id="bool-column"), + ], +) +def test_dataframe_key_columns_reach_the_native_encoder(column, monkeypatch) -> None: + """`data=df, key="id"` resolves to a Series, not an ndarray (§ routing).""" + pd = pytest.importorskip("pandas") + series = pd.Series(column) + calls: list[np.ndarray] = [] + native = k.transition_keys_fixed + + def tracked(values: np.ndarray, label: str): + calls.append(values) + return native(values, label) + + monkeypatch.setattr(k, "transition_keys_fixed", tracked) + actual = component_api._encode_transition_keys(series, len(series), "frame key") + + assert len(calls) == 1 + np.testing.assert_array_equal(actual, _python_transition_key_reference(column)) + + +def test_dataframe_key_column_with_missing_values_keeps_its_python_error() -> None: + pd = pytest.importorskip("pandas") + series = pd.Series([1.0, None, 3.0], dtype="Float64") + with pytest.raises(ValueError, match="animation key is missing at row 1"): + component_api._encode_transition_keys(series, 3, "frame key") + + +def test_native_transition_key_argument_errors_are_loud() -> None: + """A layout the kernel refuses is a bug, not a reason to degrade silently. + + Status 1 means "declined this data, use the oracle"; status 4 means the + caller sent a layout the ABI does not define. Collapsing the two would let + a `_native.py`/`valid_layout` drift turn into a silent ~7x regression that + every existing assertion still passes. + """ + with pytest.raises(ValueError, match="must be a non-object 1-D array"): + k.transition_keys_fixed(np.array(["a", "b"], dtype=object), "bad key") + with pytest.raises(ValueError, match="must use Unicode, bytes, bool, integer, or float"): + k.transition_keys_fixed(np.array(["2024-01-01"], dtype="datetime64[D]"), "bad key") + + # The wrapper's own dtype gate and Rust's `valid_layout` agree today, so + # reaching status 4 needs the seam forced open. What matters is that it + # raises rather than returning the None that means "use the oracle". + from xy import _native + + original = _native._lib.xy_transition_keys_fixed + try: + _native._lib.xy_transition_keys_fixed = lambda *_args: 4 + with pytest.raises(RuntimeError, match=r"rejected the .* layout it was handed"): + _native.transition_keys_fixed(np.array([b"ab"], dtype="S2"), "bad key") + finally: + _native._lib.xy_transition_keys_fixed = original + + def test_aggregate_tier_records_key_matching_fallback() -> None: chart = xy.scatter_chart( xy.scatter(