improvise-gk51: the pivot branch too, a shipped-document enumeration, and the argument

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze

context/code/command.md

@@ -3657,6 +3657,40 @@ The question it asks is the projection's own, never a second derivation of it:
   `Record`. A write whose whole key the host's grid does not draw is not
   refused here, as it was not before this rule existed.
 
+**The projection asks the first rule too (improvise-gk51).** The non-item rule
+asked the document, while `project::cell_id` did not ask at all. In records mode
+every measure cell of a stored record was a `Record`, including a record whose
+`Region=Mars` names no item. `set-cell`, an undeclared `## Data` item and a drill
+commit after an item was removed all make such records. So the host offered a
+write it then refused. The two ends now share one predicate,
+`Model::first_non_item`: `App::non_item_refusal` refuses with it, and
+`cell_id` declines with it, in both branches. The pivot branch needs it as well.
+A page selection naming no item (`set_page_selection` does not validate) puts
+the stale item into every cell's key, and that too was a `Record` the host
+refused.
+
+Such a cell projects as a new kind, **`CellId::NonItem { category, item }`**,
+and not as one of the existing ones. Each existing kind would lie about it:
+
+- `Aggregate` invites a drill, and its refusal says the cell "combines several
+  records". This is one stored cell.
+- `Coordinate` tells the user that renaming is not built yet.
+- `Derived` names a formula that does not exist.
+- `None` means "no cell" (`protocol.md` §3), and this cell holds a value.
+
+The variant carries the offending coordinate, so every decline says the same
+sentence: `improvise_protocol::non_item_refusal`, which the host's refusal uses
+too. The two front ends agree because the TUI's `value_cell_key` and
+`committable_cell_key` also read `cursor_cell_id`: `p`, `x` and a commit on
+such a cell are refused with that sentence in the terminal as well. A
+`NonItem` publishes no record version, since no basis can claim a write it
+could never make.
+
+Option (b) on the bead, closing the sources, is not done here. It is
+improvise-98pd. With (a) in place, a source that still writes a non-item
+produces a cell that is declined everywhere, not one that is offered and then
+refused.
+
 **Why the `vec![]` arm stays.** A reviewer asked whether a `SetCell` whose cell
 does not resolve at all, which `model_effects` turns into `vec![]` and so
 applies nothing, should also be a `Rejection`. Through a `Host`, it already is,

context/code/protocol.md

@@ -660,10 +660,16 @@ now.
 That is `Some(CellId::Aggregate(..))`, and the distinction is the whole point
 of the enum. `None` means the slot holds **no projected cell**: a `GridCache`
 position padded by `fit_to_labels`, or one blanked by a reframing. Every
-records-mode column is some kind of cell (`Record`, `Derived`, `Create` or
-`Coordinate`). Conflating the two either hides a cell or offers a
+records-mode column is some kind of cell (`Record`, `Derived`, `Create`,
+`Coordinate` or `NonItem`). Conflating the two either hides a cell or offers a
 write the type system exists to make unconstructible.
 
+`NonItem { category, item }` is a stored value whose `category` coordinate
+names no item of it (improvise-gk51). It is drawn, and it takes no write. The
+host would refuse one, so it is not offered as a `Record`. Why it is a kind of
+its own rather than one of the others: `context/code/command.md`, "The
+projection asks the first rule too".
+
 ### `CellDisplay::edit_text` — the difference corrupts data
 
 What an editor opened on this cell should start with: the value as the user

context/test-evidence/command.md

@@ -6276,3 +6276,52 @@ case fails. Second, the ticket's suggested fix, which drops the asker's
 reset and the client never heard. That rival also fails
 `a_geometry_command_relays_the_cursor_reset_to_the_asker_too` and the
 ws-server's `the_other_document_would_also_have_a_cursor_to_invalidate`.
+
+### every_projected_record_is_writable
+
+improvise-gk51, found reviewing #252 at `47b166ee`. #252's `non_item_refusal`
+refuses a wire write unless every coordinate names an item, but
+`project::cell_id` never asked. In records mode every measure cell of a stored
+record was projected as `CellId::Record`, including a record whose
+`Region=Mars` names no item. So the host offered a write and then refused it.
+
+The fixture is `records_app()` plus a stored `{Region=Mars, _Measure=Revenue}`,
+the record `set-cell` writes. The test enumerates every projected cell, takes
+the `Record`s and asks `App::unwritable` of each. The refused list must be
+empty, and it also asserts at least four `Record`s remain (East and North).
+**Red at `47b166ee`:** `(2, 2)` and `(2, 3)`, the Mars row's Revenue and Cost,
+were refused with "Mars is no item of Region, so there is no one cell to
+write".
+
+Rivals, each injected in `cell_id`:
+
+- **The records branch does not ask:** this test fails with the same two cells.
+- **Every records-mode measure cell declined (over-refusal):** this test fails
+  on "every East and North measure cell is still a Record: 0". That is the half
+  that keeps "decline everything" from passing.
+
+### every_projected_record_is_writable_on_a_stale_page
+
+improvise-gk51, the pivot branch, which the ticket did not predict.
+`View::set_page_selection` does not validate, so a selection naming no item puts
+that item into every cell's key.
+
+The fixture is Region on page, Month on row and `_Measure` on column, with one
+item each. Its control is the same workbook at a live page, where there is at
+least one `Record` and none is refused. Then the selection is set to `Mars`. The
+test asserts the grid draws a cell projected as `NonItem { Region, Mars }`, so
+the stale page is not empty, and that no projected `Record` is refused.
+
+**Rival: the pivot branch does not ask.** This test fails at the `NonItem`
+assertion. With that assertion removed, it fails with
+`(0, 0, "Mars is no item of Region, …")`, so the pivot branch had the ticket's
+bug too.
+
+### every_record_a_shipped_document_projects_is_writable
+
+improvise-gk51's enumeration over real documents. It takes every tracked
+`.improv` in the pivot view it opens in, then again in records mode (`R`), and
+asserts that every projected `Record` is writable. It passes at the base too:
+the reviewer counted 518 cells across the shipped documents and found no
+non-item record among them. So it is a **non-regression guard**, not a
+reproduction; the two fixtures above are the reproductions.

crates/improvise-command/src/ui/project.rs

@@ -5300,8 +5300,26 @@ mod cell_identity {
         }
     }
 
-    /// improvise-gk51: a cell projected as a `Record` is one a wire write may land
-    /// on. Evidence: `context/test-evidence/command.md`.
+    /// Every cell `app` projects as a `Record`, and those the host then refuses
+    /// a write to. The refused list must be empty (improvise-gk51).
+    fn records_and_refusals(app: &App) -> (usize, Vec<(usize, usize, String)>) {
+        let grid = grid_of(app, WHOLE);
+        let records: Vec<(usize, usize, RecordCell)> = (0..app.layout.row_count())
+            .flat_map(|r| (0..app.layout.col_count()).map(move |c| (r, c)))
+            .filter_map(|(r, c)| match grid.cell(r, c)?.id.clone()? {
+                CellId::Record(record) => Some((r, c, record)),
+                _ => None,
+            })
+            .collect();
+        let refused = records
+            .iter()
+            .filter_map(|(r, c, record)| app.unwritable(record).map(|why| (*r, *c, why)))
+            .collect();
+        (records.len(), refused)
+    }
+
+    /// improvise-gk51: a records-mode cell projected as a `Record` is one a wire
+    /// write may land on. Evidence: `context/test-evidence/command.md`.
     #[test]
     fn every_projected_record_is_writable() {
         let mut app = records_app();
@@ -5315,29 +5333,52 @@ mod cell_identity {
         );
         app.rebuild_layout();
         let grid = grid_of(&app, WHOLE);
-        let cells: Vec<(usize, usize, CellDisplay)> = (0..app.layout.row_count())
+        let drawn = (0..app.layout.row_count())
             .flat_map(|r| (0..app.layout.col_count()).map(move |c| (r, c)))
-            .filter_map(|(r, c)| grid.cell(r, c).map(|cell| (r, c, cell.clone())))
-            .collect();
+            .any(|(r, c)| grid.cell(r, c).is_some_and(|cell| cell.text == "Mars"));
+        assert!(drawn, "setup: the non-item record is drawn");
+        let (records, refused) = records_and_refusals(&app);
         assert!(
-            cells.iter().any(|(_, _, cell)| cell.text == "Mars"),
-            "setup: the non-item record is drawn"
+            records >= 4,
+            "every East and North measure cell is still a Record: {records}"
         );
-        let records: Vec<(usize, usize, RecordCell)> = cells
-            .iter()
-            .filter_map(|(r, c, cell)| match &cell.id {
-                Some(CellId::Record(record)) => Some((*r, *c, record.clone())),
-                _ => None,
-            })
-            .collect();
         assert!(
-            records.len() >= 4,
-            "every East and North measure cell is still a Record: {records:?}"
+            refused.is_empty(),
+            "projected as Record, then refused: {refused:?}"
         );
-        let refused: Vec<(usize, usize, String)> = records
-            .into_iter()
-            .filter_map(|(r, c, record)| app.unwritable(&record).map(|why| (r, c, why)))
-            .collect();
+    }
+
+    /// improvise-gk51, the pivot branch: a page selection naming no item puts a
+    /// non-item in every cell's key. Evidence: `context/test-evidence/command.md`.
+    #[test]
+    fn every_projected_record_is_writable_on_a_stale_page() {
+        let mut wb = Workbook::new("Pivot");
+        wb.add_category("Region").unwrap();
+        wb.add_category("Month").unwrap();
+        wb.add_item("Region", "East").unwrap();
+        wb.add_item("Month", "Jan").unwrap();
+        wb.add_item("_Measure", "Revenue").unwrap();
+        let view = wb.active_view_mut();
+        view.set_axis("Region", Axis::Page);
+        view.set_axis("Month", Axis::Row);
+        view.set_axis("_Measure", Axis::Column);
+        let control = App::new(wb.clone(), None);
+        let (records, refused) = records_and_refusals(&control);
+        assert!(
+            records >= 1 && refused.is_empty(),
+            "setup: a live page is a Record"
+        );
+        wb.active_view_mut().set_page_selection("Region", "Mars");
+        let app = App::new(wb, None);
+        let named = grid_of(&app, WHOLE).cells.iter().flatten().any(|cell| {
+            cell.id
+                == Some(CellId::NonItem {
+                    category: "Region".into(),
+                    item: "Mars".into(),
+                })
+        });
+        assert!(named, "the stale page's cells are drawn, and named for it");
+        let (_, refused) = records_and_refusals(&app);
         assert!(
             refused.is_empty(),
             "projected as Record, then refused: {refused:?}"

tests/shipped_documents_load.rs

@@ -50,6 +50,51 @@ fn every_shipped_document_loads() {
     }
 }
 
+/// improvise-gk51: in every shipped document, pivot and records mode, a cell
+/// projected as a `Record` is one the host would accept a write to.
+#[test]
+fn every_record_a_shipped_document_projects_is_writable() {
+    use improvise::command::key::{KeyCode, KeyEvent, KeyModifiers};
+    use improvise::protocol::CellId;
+    use improvise::ui::app::App;
+    use improvise_command::ui::project::{Viewport, grid_of};
+
+    let root = Path::new(env!("CARGO_MANIFEST_DIR"));
+    let mut records = 0;
+    let mut refused = Vec::new();
+    for path in shipped_documents(root) {
+        let workbook = improvise::persistence::load(&path).expect("every shipped document loads");
+        let mut app = App::new(workbook, None);
+        for _mode in ["pivot", "records"] {
+            let whole = Viewport {
+                row_offset: 0,
+                col_offset: 0,
+                rows: app.layout.row_count(),
+                cols: app.layout.col_count(),
+            };
+            let grid = grid_of(&app, whole);
+            for cell in grid.cells.iter().flatten() {
+                if let Some(CellId::Record(record)) = &cell.id {
+                    records += 1;
+                    if let Some(why) = app.unwritable(record) {
+                        refused.push(format!("{}: {why}", path.display()));
+                    }
+                }
+            }
+            let r = KeyEvent::new(KeyCode::Char('R'), KeyModifiers::NONE);
+            let _ = app.handle_key(r);
+        }
+    }
+    assert!(
+        records > 0,
+        "precondition: the shipped documents project records"
+    );
+    assert!(
+        refused.is_empty(),
+        "projected as Record, then refused: {refused:?}"
+    );
+}
+
 /// A scratch repository, so each way of shipping a document is set up for real.
 fn scratch_repo() -> tempfile::TempDir {
     let dir = tempfile::tempdir().expect("a temp dir");