Merge pull request #173 from fiddlerwoaroof/fix/df2w-i0v9-browser-page

df2w: an open, empty formula panel is drawn in the browser

context/code/clients.md

@@ -1491,6 +1491,19 @@ what says it is closed**. That is the shape `col_groups` already uses for "this
 grid has no column groups" — an absent list rather than a flag the page has to
 interpret alongside a mode it would then have to understand.
 
+**That shape is only sound where "open" implies "non-empty"**, and help is the
+one place it does — its content is static. `views` and `formulas` borrowed it
+and were wrong: a document with no formulas opened a formula panel that built
+the same `RenderModel` as a closed one, so the page hid it under a mode reading
+FORMULAS (improvise-df2w). Both are now `Option<Vec<_>>` — `None` closed,
+`Some(vec![])` open and empty — one field, so the two facts cannot disagree,
+where a `formula_panel_open` flag beside the rows (the `category_panel_open`
+shape) would have been two. The same mistake as improvise-xg1z: an absence
+spent on encoding a second fact. What the page does **not** yet draw is the
+TUI's empty-state line (`(no formulas — press 'n' to add)`); that string lives
+in `src/ui/formula_panel.rs`, and copying it into `render.rs` would be a second
+home for it.
+
 The rows themselves are `improvise_protocol::help::pages()`, the same ones
 `src/ui/help.rs` draws. `RenderHelpRow` carries a `kind` string
 (`heading`, `keys`, `accent`, …) which is a *semantic* name the page maps to a

context/test-evidence/clients.md

@@ -1130,8 +1130,8 @@ Normal and never closes the panel, exactly as `src/draw.rs` keeps drawing it
 off `view_panel_open` alone. Asserting it hidden would have pinned the browser
 to behaviour the TUI does not have.
 
-The gating rule itself — an empty list is what says a panel is closed, the same
-rule `help` follows — is asserted in Rust as well, by
+The gating rule itself — `None` is what says a panel is closed (it was an empty
+list, the rule `help` follows, until improvise-df2w) — is asserted in Rust as well, by
 `a_closed_panel_projects_no_rows_and_an_open_one_does` and
 `the_tile_bar_is_always_present_and_only_its_cursor_is_modal`, because a browser
 test cannot distinguish "closed" from "the cache is empty" and those two tests
@@ -4231,6 +4231,33 @@ and converged.
 **Fault injected:** the same `Binding::Prefix` → `Upstream` reversion. Fails on
 the second press.
 
+### `an_open_empty_panel_is_not_a_closed_one`
+
+improvise-df2w. `views` and `formulas` said "closed" by being empty, so an open
+formula panel over a document with no formulas built a `RenderModel` identical
+to a closed one, and the page hid it while the mode read FORMULAS. The test
+compares the two models whole rather than asserting a field, so it does not
+depend on how the difference is spelled.
+
+**Reproduced:** fails at HEAD before the fix with `left == right` on the formula
+half — the two models were byte-identical. The existing
+`a_closed_panel_projects_no_rows_and_an_open_one_does` could not see it, because
+it fills both caches before opening the panels.
+
+### the formula panel opens over a document with no formulas
+
+The browser half of improvise-df2w, in `harness/tests/contract.spec.js`, over
+`harness/fixtures/tall.improv`, which has no `## Formulas` section. It asserts
+**both** directions, because each rival fails a different one.
+
+**Reproduced:** against a bundle built from the unfixed `render.rs` and
+`app.js`, fails at `#formulas` `toBeVisible()`, immediately after `#mode` reads
+FORMULAS — the exact disagreement the ticket describes.
+
+**Fault injected:** `paintPanel` writing `host.hidden = false` (a page that
+stops inferring closed and simply never hides). Fails at the launch-time
+`toBeHidden()` before the panel is opened.
+
 ## improvise-ktlk — the overlay and the commit read one buffer
 
 PR #162 (improvise-xg1z) changed what the overlay **draws** for an `Editing`

crates/improvise-wasm-client/src/render.rs

@@ -42,11 +42,11 @@ pub struct RenderModel {
     pub categories: Vec<RenderCatRow>,
     /// Whether the panel is open, so the page does not decide.
     pub category_panel_open: bool,
-    /// The view panel's rows, **empty unless it is open** — the same rule the
-    /// help overlay follows, so the page asks no question about the mode.
-    pub views: Vec<RenderViewRow>,
-    /// The formula panel's rows, empty unless it is open.
-    pub formulas: Vec<RenderFormulaRow>,
+    /// The view panel's rows, **`None` when it is closed** and possibly empty
+    /// when open: an absence is not an empty list (improvise-df2w).
+    pub views: Option<Vec<RenderViewRow>>,
+    /// The formula panel's rows, on the same terms as `views`.
+    pub formulas: Option<Vec<RenderFormulaRow>>,
     /// The tile bar, drawn on every frame as the TUI draws it. Which tile is
     /// `selected` is the mode's answer, not the page's.
     pub tiles: Vec<RenderTile>,
@@ -341,41 +341,39 @@ fn page_axes(state: &ClientState) -> Vec<RenderPageAxis> {
         .collect()
 }
 
-/// The view panel's rows while it is open. The axis line is
+/// The view panel's rows, or `None` while it is closed. The axis line is
 /// `viewmodel::view_axis_summary`, which the TUI's own panel calls too.
-fn view_rows(state: &ClientState) -> Vec<RenderViewRow> {
-    if !state.view.view_panel_open {
-        return Vec::new();
-    }
-    state
-        .views
-        .views
-        .iter()
-        .enumerate()
-        .map(|(i, summary)| RenderViewRow {
-            name: summary.name.clone(),
-            axes: improvise_protocol::viewmodel::view_axis_summary(summary),
-            active: summary.name == state.view.active_view,
-            selected: i == state.view.view_panel_cursor,
-        })
-        .collect()
+fn view_rows(state: &ClientState) -> Option<Vec<RenderViewRow>> {
+    state.view.view_panel_open.then(|| {
+        state
+            .views
+            .views
+            .iter()
+            .enumerate()
+            .map(|(i, summary)| RenderViewRow {
+                name: summary.name.clone(),
+                axes: improvise_protocol::viewmodel::view_axis_summary(summary),
+                active: summary.name == state.view.active_view,
+                selected: i == state.view.view_panel_cursor,
+            })
+            .collect()
+    })
 }
 
-/// The formula panel's rows while it is open.
-fn formula_rows(state: &ClientState) -> Vec<RenderFormulaRow> {
-    if !state.view.formula_panel_open {
-        return Vec::new();
-    }
-    state
-        .formulas
-        .formulas
-        .iter()
-        .enumerate()
-        .map(|(i, text)| RenderFormulaRow {
-            text: text.clone(),
-            selected: i == state.view.formula_cursor,
-        })
-        .collect()
+/// The formula panel's rows, or `None` while it is closed.
+fn formula_rows(state: &ClientState) -> Option<Vec<RenderFormulaRow>> {
+    state.view.formula_panel_open.then(|| {
+        state
+            .formulas
+            .formulas
+            .iter()
+            .enumerate()
+            .map(|(i, text)| RenderFormulaRow {
+                text: text.clone(),
+                selected: i == state.view.formula_cursor,
+            })
+            .collect()
+    })
 }
 
 /// The tile bar, always. Only the *selection* is conditional, exactly as
@@ -797,34 +795,48 @@ mod tests {
         client
     }
 
-    /// An empty list is what says a panel is closed — the same rule the help
-    /// overlay follows, so the page asks no question about the mode.
+    /// `None` is what says a panel is closed, so the page asks no question
+    /// about the mode.
     #[test]
     fn a_closed_panel_projects_no_rows_and_an_open_one_does() {
         let mut client = client_with_panels();
         let closed = client.model();
         assert!(
-            closed.views.is_empty(),
+            closed.views.is_none(),
             "the cache is full but the panel is shut"
         );
-        assert!(closed.formulas.is_empty());
+        assert!(closed.formulas.is_none());
 
         client.state_mut().view.view_panel_open = true;
         client.state_mut().view.formula_panel_open = true;
         client.state_mut().view.view_panel_cursor = 1;
         client.state_mut().view.active_view = "Default".into();
         let open = client.model();
-        assert_eq!(open.views.len(), 2);
+        let (views, formulas) = (open.views.unwrap(), open.formulas.unwrap());
+        assert_eq!(views.len(), 2);
         assert_eq!(
-            open.views[0].axes, "R:Region",
+            views[0].axes, "R:Region",
             "the axis line is the shared derivation, and _Dim is not part of it"
         );
-        assert!(
-            open.views[0].active,
-            "active is ViewState's, not the cache's"
-        );
-        assert!(open.views[1].selected, "and selected is the panel cursor's");
-        assert_eq!(open.formulas[0].text, "Profit = Revenue - Cost");
+        assert!(views[0].active, "active is ViewState's, not the cache's");
+        assert!(views[1].selected, "and selected is the panel cursor's");
+        assert_eq!(formulas[0].text, "Profit = Revenue - Cost");
+    }
+
+    /// Open with nothing in it is not closed: the page must be able to tell
+    /// them apart from the model alone (improvise-df2w).
+    #[test]
+    fn an_open_empty_panel_is_not_a_closed_one() {
+        let mut client = init();
+        let closed = client.model();
+        client.state_mut().view.formula_panel_open = true;
+        let formulas_open = client.model();
+        assert_ne!(formulas_open, closed, "an open, empty formula panel");
+
+        client.state_mut().view.formula_panel_open = false;
+        client.state_mut().view.view_panel_open = true;
+        let views_open = client.model();
+        assert_ne!(views_open, closed, "an open, empty view panel");
     }
 
     /// The tile bar is drawn always; only the *selection* waits for the mode,

harness/tests/contract.spec.js

@@ -70,7 +70,7 @@ test.describe("every field the render model projects reaches the screen", () =>
     await expect(page.locator("#categories")).toBeVisible();
     await page.keyboard.press("Escape");
 
-    // `views`: empty unless open, so the page asks no question about the mode.
+    // `views`: `null` unless open, so the page asks no question about the mode.
     await expect(page.locator("#views")).toBeHidden();
     await page.keyboard.press("Shift+V");
     await expect(page.locator("#views li")).toHaveCount(2);
@@ -356,6 +356,27 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
   });
 });
 
+test.describe("an open panel is drawn even when it has nothing in it", () => {
+  // improvise-df2w: `formulas` said "closed" by being empty, so a document with
+  // no formulas could open the panel and never see it. `tall.improv` has none.
+  test("the formula panel opens over a document with no formulas", async ({ page }) => {
+    await page.goto("/app/index.html?doc=df2w-empty.improv&seed=tall.improv");
+    await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 });
+    await page.locator("#grid").focus();
+    await expect(page.locator("#formulas")).toBeHidden();
+
+    await page.keyboard.press("Shift+F");
+    await expect(page.locator("#mode")).toHaveText("FORMULAS");
+    await expect(page.locator("#formulas")).toBeVisible();
+    await expect(page.locator("#formulas li")).toHaveCount(0);
+
+    // And closed is still closed: the other direction, which a page that
+    // simply never hid the panel would fail.
+    await page.keyboard.press("Shift+F");
+    await expect(page.locator("#formulas")).toBeHidden();
+  });
+});
+
 test.describe("the selection is on screen", () => {
   // improvise-jq8l: `web/app.js` called `scrollIntoView` NOWHERE, so a selection
   // moved past the bottom of `#grid` simply stayed there, invisible, with no way

web/app.js

@@ -594,12 +594,12 @@ function edit(td, row, col, seed) {
   });
 }
 
-/** A side panel. An empty row list is what says it is closed — the page asks
- *  no question about the mode, exactly as `paintHelp` does not. */
+/** A side panel. `null` is what says it is closed, and `[]` is open and empty:
+ *  two facts, never inferred from a count (improvise-df2w). */
 function paintPanel(host, rows, build) {
-  host.hidden = rows.length === 0;
+  host.hidden = rows === null;
   const list = document.createElement("ul");
-  rows.forEach((row, index) => {
+  (rows ?? []).forEach((row, index) => {
     const li = build(row);
     li.dataset.index = String(index);
     list.appendChild(li);