diff --git a/context/test-evidence/core.md b/context/test-evidence/core.md index e553a469..55543e64 100644 --- a/context/test-evidence/core.md +++ b/context/test-evidence/core.md @@ -2115,3 +2115,10 @@ The TUI item row. Not separately injected: its draw path is the one line the Rival: the column header uses `col_label` (the key). The exact output pins the corner cell `Kind (Who=Alicia)`, the column header and the row label. + +### view::layout::tests::a_page_axis_with_ragged_displays_is_refused + +From Copilot's finding on #336: `item_displays` is a parallel list on a wire type +that derives `Deserialize`. The control parses a well-formed axis first; then a +shorter list and an absent one beside `items: Some(..)` are each refused with the +"item displays" message. Without the `try_from` the same JSON parses. diff --git a/crates/improvise-core/src/view/layout.rs b/crates/improvise-core/src/view/layout.rs index df9e0387..37a0c133 100644 --- a/crates/improvise-core/src/view/layout.rs +++ b/crates/improvise-core/src/view/layout.rs @@ -490,6 +490,7 @@ fn innermost_group(items: &[AxisEntry], idx: usize) -> Option<(String, String)> /// One category constraining which records are visible: what it is fixed to, /// and — when the user may page it — what `[`/`]` cycle. core.md §7. #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(try_from = "UncheckedPageAxis")] pub struct PageAxis { pub cat: String, /// What the bar shows for `cat`; the key stays what a command names (core.md §21). @@ -505,6 +506,41 @@ pub struct PageAxis { pub item_displays: Option>, } +/// A [`PageAxis`] as the wire delivers it, before `item_displays` is checked +/// against `items`: a parallel list a client must not see ragged. +#[derive(serde::Deserialize)] +struct UncheckedPageAxis { + cat: String, + cat_display: String, + selected: String, + selected_display: String, + items: Option>, + item_displays: Option>, +} + +impl TryFrom for PageAxis { + type Error = String; + + fn try_from(raw: UncheckedPageAxis) -> Result { + let keys = raw.items.as_ref().map(Vec::len); + let shown = raw.item_displays.as_ref().map(Vec::len); + match keys == shown { + true => Ok(Self { + cat: raw.cat, + cat_display: raw.cat_display, + selected: raw.selected, + selected_display: raw.selected_display, + items: raw.items, + item_displays: raw.item_displays, + }), + false => Err(format!( + "page axis {}: {keys:?} items but {shown:?} item displays", + raw.cat + )), + } + } +} + /// The page axes as the plain coordinate pairs `matching_cells` and /// `CellKey::new` take. pub fn page_constraints(axes: &[PageAxis]) -> Vec<(String, String)> { @@ -1192,7 +1228,7 @@ impl GridEval<'_> { #[cfg(test)] mod tests { use super::{ - AxisEntry, GridLayout, NEW_CATEGORY_COL, NEW_MEASURE_COL, RecordCol, data_items, + AxisEntry, GridLayout, NEW_CATEGORY_COL, NEW_MEASURE_COL, PageAxis, RecordCol, data_items, prune_axis_items, synthetic_record_info, }; use crate::model::Aggregation; @@ -1736,6 +1772,22 @@ mod tests { ); } + #[test] + fn a_page_axis_with_ragged_displays_is_refused() { + let good = PageAxis::plain("Q", "Q1", Some(vec!["Q1".into(), "Q2".into()])); + let wire = serde_json::to_value(&good).expect("a page axis serializes"); + assert_eq!( + serde_json::from_value::(wire.clone()).unwrap(), + good + ); + for bad in [serde_json::json!(["Only one"]), serde_json::json!(null)] { + let mut ragged = wire.clone(); + ragged["item_displays"] = bad; + let err = serde_json::from_value::(ragged).unwrap_err(); + assert!(err.to_string().contains("item displays"), "{err}"); + } + } + #[test] fn a_folded_group_is_shown_under_the_groups_display_and_keyed_by_its_name() { let mut wb = shown_otherwise();