From 8d3230e68535c78b4e1b56a96255f0cc4a94fae2 Mon Sep 17 00:00:00 2001 From: "Claude (agent-hier)" Date: Thu, 24 Sep 2026 22:29:02 -0700 Subject: [PATCH] fix(core): an expanded fold's depth is non-zero by type, so every fold the API builds reads back (improvise-felo.6) Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/test-evidence/formula-and-io.md | 11 +++++++ crates/improvise-core/src/view/types.rs | 19 +++++++---- crates/improvise-io/src/persistence/format.rs | 2 +- crates/improvise-io/src/persistence/parse.rs | 6 ++-- .../src/persistence/roundtrip_tests.rs | 32 +++++++++++++++++-- tests/grid_frame_agreement.rs | 6 +++- 6 files changed, 63 insertions(+), 13 deletions(-) diff --git a/context/test-evidence/formula-and-io.md b/context/test-evidence/formula-and-io.md index 717ba987..ca3a425d 100644 --- a/context/test-evidence/formula-and-io.md +++ b/context/test-evidence/formula-and-io.md @@ -1648,3 +1648,14 @@ category named `expanded`. closed, the one its `collapsed:` line names and the three it does not, and the line is written back. Rival: the old open default; this test fails with the other eleven in that rival's list. + +### every_fold_the_api_can_build_reads_back_from_the_file_it_writes + +ChatGPT Codex, P2 on #317 at `e09ec752`: `GroupFold::Expanded(usize)` and +`View::set_group_fold` are public, so `Expanded(0)` could be built, written as +`expanded(0): …` and then refused by the grammar that wrote it. Confirmed by +reading: the writer formatted any `usize`, and `depth_number` starts at `1`. +The fix makes the state unrepresentable (`Expanded(NonZeroUsize)`), so the +regression guard is the type, and the old failing input no longer compiles. +This test covers every fold the API CAN build (`Collapsed` and depths 1 to 3, +beside a category depth), each saved and read back. diff --git a/crates/improvise-core/src/view/types.rs b/crates/improvise-core/src/view/types.rs index c6e8b50b..448ff94c 100644 --- a/crates/improvise-core/src/view/types.rs +++ b/crates/improvise-core/src/view/types.rs @@ -1,5 +1,6 @@ use indexmap::IndexMap; use std::collections::{HashMap, HashSet}; +use std::num::NonZeroUsize; use super::axis::Axis; use crate::model::category::CategoryKind; @@ -32,8 +33,9 @@ pub fn unused_view_name<'a>(existing: impl IntoIterator) -> Stri pub enum GroupFold { /// `collapsed: Cat/Group` — closed, whatever reaches it from above. Collapsed, - /// `expanded(N): Cat/Group` — open, with N-1 levels beneath it; N >= 1. - Expanded(usize), + /// `expanded(N): Cat/Group` — open, with N-1 levels beneath it. Non-zero by + /// type, since `expanded(0)` is no spelling the grammar reads back. + Expanded(NonZeroUsize), } #[derive(Clone)] @@ -273,7 +275,7 @@ impl View { let depth = chain.len().saturating_sub(1); let by_ancestor = chain[..depth].iter().enumerate().any(|(at, ancestor)| { matches!(self.group_fold(cat_name, ancestor), - Some(GroupFold::Expanded(n)) if depth - at < n) + Some(GroupFold::Expanded(n)) if depth - at < n.get()) }); !by_ancestor && depth >= self.expand_depth(cat_name) } @@ -310,7 +312,7 @@ impl View { false => { let fold = match collapse { true => GroupFold::Collapsed, - false => GroupFold::Expanded(1), + false => GroupFold::Expanded(NonZeroUsize::MIN), }; self.set_group_fold(cat_name, group, fold); } @@ -371,6 +373,7 @@ mod tests { use super::{GroupFold, View, unused_view_name}; use crate::model::category::CategoryKind; use crate::view::Axis; + use std::num::NonZeroUsize; /// improvise-vw47: the count is the starting guess, never the answer. /// What a taken name costs: `context/test-evidence/core.md`. @@ -664,7 +667,11 @@ mod tests { #[test] fn a_groups_depth_opens_it_and_n_minus_one_levels_beneath() { let mut v = View::new("Test"); - v.set_group_fold("Time", "2024", GroupFold::Expanded(2)); + v.set_group_fold( + "Time", + "2024", + GroupFold::Expanded(NonZeroUsize::new(2).unwrap()), + ); assert!(!v.is_group_collapsed("Time", &["2024"])); assert!(!v.is_group_collapsed("Time", &["2024", "Q1"])); assert!(v.is_group_collapsed("Time", &["2024", "Q1", "Jan"])); @@ -690,7 +697,7 @@ mod tests { v.toggle_group_collapse("Time", &["2024", "Q1-2024"]); assert_eq!( v.group_fold("Time", "Q1-2024"), - Some(GroupFold::Expanded(1)) + Some(GroupFold::Expanded(NonZeroUsize::MIN)) ); v.toggle_group_collapse("Time", &["2024"]); assert_eq!(v.group_fold("Time", "2024"), Some(GroupFold::Collapsed)); diff --git a/crates/improvise-io/src/persistence/format.rs b/crates/improvise-io/src/persistence/format.rs index b20ea859..39046be4 100644 --- a/crates/improvise-io/src/persistence/format.rs +++ b/crates/improvise-io/src/persistence/format.rs @@ -148,7 +148,7 @@ pub fn format_md(workbook: &Workbook) -> String { let target = format!("{}/{}", quote_name(cat), quote_name(group)); match fold { GroupFold::Collapsed => w!(out, "collapsed: {target}"), - GroupFold::Expanded(1) => w!(out, "expanded: {target}"), + GroupFold::Expanded(n) if n.get() == 1 => w!(out, "expanded: {target}"), GroupFold::Expanded(n) => w!(out, "expanded({n}): {target}"), } } diff --git a/crates/improvise-io/src/persistence/parse.rs b/crates/improvise-io/src/persistence/parse.rs index d92e19cd..e2661cf7 100644 --- a/crates/improvise-io/src/persistence/parse.rs +++ b/crates/improvise-io/src/persistence/parse.rs @@ -399,7 +399,7 @@ fn walk_axis_line(entry: Pair<'_, Rule>, pv: &mut PView) -> Result<()> { /// `expanded(N): Cat` or `expanded(N): Cat/Group`; a bare `expanded:` is N = 1. fn walk_expanded_line(entry: Pair<'_, Rule>) -> Result { - let mut depth = 1usize; + let mut depth = std::num::NonZeroUsize::MIN; let mut names = Vec::new(); for part in entry.into_inner() { match part.as_rule() { @@ -557,8 +557,8 @@ fn build_view(wb: &mut Workbook, pv: &PView) -> Result<()> { _ => view.set_group_fold(category, g, *fold), }, (None, GroupFold::Expanded(n)) => match view.expand_depth(category) { - 0 => view.set_expand_depth(category, *n), - held if held != *n => return Err(conflict().into()), + 0 => view.set_expand_depth(category, n.get()), + held if held != n.get() => return Err(conflict().into()), _ => {} }, (None, GroupFold::Collapsed) => bail!("a category-wide collapse has no spelling"), diff --git a/crates/improvise-io/src/persistence/roundtrip_tests.rs b/crates/improvise-io/src/persistence/roundtrip_tests.rs index d565723a..4c2c7c69 100644 --- a/crates/improvise-io/src/persistence/roundtrip_tests.rs +++ b/crates/improvise-io/src/persistence/roundtrip_tests.rs @@ -305,9 +305,12 @@ fn expanded_and_collapsed_lines_round_trip_together() { assert_eq!(v.group_fold("Date", "Q1-2024"), Some(GroupFold::Collapsed)); assert_eq!( v.group_fold("Date", "Q2-2024"), - Some(GroupFold::Expanded(1)) + Some(GroupFold::Expanded(std::num::NonZeroUsize::MIN)) + ); + assert_eq!( + v.group_fold("Date", "2025"), + Some(GroupFold::Expanded(std::num::NonZeroUsize::new(3).unwrap())) ); - assert_eq!(v.group_fold("Date", "2025"), Some(GroupFold::Expanded(3))); let written = format_md(&loaded); for line in [ "expanded(2): Date", @@ -1423,3 +1426,28 @@ fn a_loaded_label_category_is_not_on_a_pivot_axis() { ); } } + +/// ChatGPT Codex on #317: a fold built through the public API must survive its +/// own save. `Expanded(0)` is now unconstructible; every depth that is survives. +#[test] +fn every_fold_the_api_can_build_reads_back_from_the_file_it_writes() { + use crate::view::GroupFold; + use std::num::NonZeroUsize; + let base = parse_md(&dates_with("")).unwrap(); + for n in 1..=3 { + let depth = NonZeroUsize::new(n).unwrap(); + for fold in [GroupFold::Collapsed, GroupFold::Expanded(depth)] { + let mut wb = base.clone(); + wb.active_view_mut().set_group_fold("Date", "2024", fold); + wb.active_view_mut().set_expand_depth("Date", n); + let text = format_md(&wb); + let back = parse_md(&text).unwrap_or_else(|e| panic!("{e}\n{text}")); + assert_eq!( + back.active_view().group_fold("Date", "2024"), + Some(fold), + "{text}" + ); + assert_eq!(back.active_view().expand_depth("Date"), n, "{text}"); + } + } +} diff --git a/tests/grid_frame_agreement.rs b/tests/grid_frame_agreement.rs index 8ab0da8e..12364556 100644 --- a/tests/grid_frame_agreement.rs +++ b/tests/grid_frame_agreement.rs @@ -215,7 +215,11 @@ fn a_date_hierarchy() -> Workbook { view.set_axis("Date", Axis::Row); view.set_axis("_Measure", Axis::Column); view.set_expand_depth("Recipient", 1); - view.set_group_fold("Date", "Y24", GroupFold::Expanded(2)); + view.set_group_fold( + "Date", + "Y24", + GroupFold::Expanded(std::num::NonZeroUsize::new(2).unwrap()), + ); view.set_group_fold("Date", "Q2", GroupFold::Collapsed); for r in ["Alice", "Carol"] { for d in ["Jan", "Feb", "Apr", "Jan25", "Undated"] {