From 942211b921ea7d96ca2158079c879f49d7ebb871 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Tue, 22 Sep 2026 20:54:26 -0700 Subject: [PATCH] df2w: an open, empty formula panel is drawn views/formulas were empty-means-closed, so an open panel over a document with no formulas built the same RenderModel as a closed one. Option> carries both facts in one field; the page hides on null. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/code/clients.md | 13 +++++++++++++ context/test-evidence/clients.md | 31 +++++++++++++++++++++++++++++-- harness/tests/contract.spec.js | 23 ++++++++++++++++++++++- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/context/code/clients.md b/context/code/clients.md index c9daea61..39124a17 100644 --- a/context/code/clients.md +++ b/context/code/clients.md @@ -1476,6 +1476,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>` — `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 diff --git a/context/test-evidence/clients.md b/context/test-evidence/clients.md index fd39ad65..8df1e45d 100644 --- a/context/test-evidence/clients.md +++ b/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 @@ -4177,3 +4177,30 @@ 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. diff --git a/harness/tests/contract.spec.js b/harness/tests/contract.spec.js index 34dd6743..718a4121 100644 --- a/harness/tests/contract.spec.js +++ b/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