test(harness): the help overlay is asserted as content, and a lost click guard returns

PR #61 deleted "? opens help, and its rows are the shared ones" and nothing replaced it (improvise-78v6): paintHelp, help, help_page and help_titles reached no assertion at all. What contract.spec.js carried instead said only that a dt was VISIBLE, and an empty dt is a visible dt (improvise-igxp). help.spec.js reads the <dl> as the pairs it is and asserts text: the five titles, Welcome's prose rows (which carry no dd at all, by design), and three key/description pairings off the keymap. The second test asks whether the string is TRUE rather than present -- it reads the key for "Jump to last row" out of the DOM, closes the overlay, presses it and asserts the selection, so the spec holds no second copy of the keymap. contract.spec.js's two help lines are strengthened from toBeVisible to text. Injecting the fault igxp names -- blanking keys.textContent in paintHelp -- left all three contract tests passing before; it now fails that file and both tests here. The test-name diff 78v6 asks for finds a fourth loss: "a click on an item row filters, as Enter does". Only #status distinguishes the item arm of click_category_row from the category arm, so a click wired to the wrong one passed the whole file. Restored, and proved by injecting the mis-wiring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
2026-09-20 16:34:18 -07:00
parent 00f7678822
commit ba5c0ee059
2 changed files with 152 additions and 2 deletions
@@ -74,13 +74,19 @@ test.describe("every field the render model projects reaches the screen", () =>
    await page.keyboard.press("Escape");

    // `help`, `help_page`, `help_titles`: the rows and their keys are
-    // `improvise_protocol::help`'s, the same ones the TUI draws.
+    // `improvise_protocol::help`'s, the same ones the TUI draws. Asserted by
+    // TEXT, because an empty `dt` is a visible `dt` and a page drawing the
+    // right number of blank rows is the failure this row is here to catch
+    // (improvise-igxp). What each page holds is `help.spec.js`.
    await expect(page.locator("#help")).toBeHidden();
    await page.keyboard.press("?");
    await expect(page.locator(".help-tab.active")).toHaveText("Welcome");
-    await expect(page.locator(".help-body dt").first()).toBeVisible();
+    await expect(page.locator(".help-body dt.help-banner")).toHaveText("improvise");
    await page.keyboard.press("l");
    await expect(page.locator(".help-tab.active")).toHaveText("Navigation");
+    await expect(page.locator(".help-body dt.help-keys").first()).toHaveText(
+      "h j k l ← ↑ → ↓",
+    );
    await page.keyboard.press("Escape");
    await expect(page.locator("#help")).toBeHidden();

@@ -168,6 +174,15 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
      "Cold Brew",
      "Pastry",
    ]);
+    // The OTHER arm of the same click, which PR #61 dropped with
+    // `pointer.spec.js` and nothing replaced (found by the test-name diff
+    // improvise-78v6 asks for): on an ITEM row the rule is "do what Enter
+    // does here", which is filter — not expand. Only the status line says
+    // which arm ran, so a click wired to the category arm passes everything
+    // above and fails here.
+    await page.locator("#categories li.item").filter({ hasText: "Latte" }).click();
+    await expect(page.locator("#status")).toContainText("Filter: Product = Latte");
+    await expect(page.locator("#categories li.selected")).toHaveText("Latte");
    await page.keyboard.press("Escape");

    // A view row (improvise-iioi). The activation must read the CLICKED index,
@@ -0,0 +1,135 @@
+// The help overlay: the rows are `improvise_protocol::help`'s, and the keys in
+// them came off the keymap (improvise-ktv1). PR #61 deleted the spec that said
+// so and nothing replaced it (improvise-78v6); what replaced it said only that
+// a row EXISTED, which a row drawn blank satisfies (improvise-igxp).
+
+import { test, expect } from "@playwright/test";
+
+async function openHelp(page) {
+  await page.goto("/app/index.html");
+  await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 });
+  await page.locator("#grid").focus();
+  await page.keyboard.press("?");
+  await expect(page.locator("#help")).toBeVisible();
+  await expect(page.locator(".help-tab.active")).toHaveText("Welcome");
+}
+
+/// The `<dl>` read as the pairs it is: a `dt` and the `dd` that follows it,
+/// which is the association the page encodes in document order and nowhere
+/// else. Reading `dt` and `dd` as two lists would zip prose rows (which have
+/// no `dd`) against the wrong descriptions and still look tidy.
+const pairs = (page) =>
+  page.locator(".help-body").evaluate((dl) =>
+    [...dl.children]
+      .filter((el) => el.tagName === "DT")
+      .map((dt) => ({
+        kind: dt.className,
+        keys: dt.textContent,
+        description:
+          dt.nextElementSibling?.tagName === "DD"
+            ? dt.nextElementSibling.textContent
+            : null,
+      })),
+  );
+
+test.describe("the help overlay draws the shared content", () => {
+  // improvise-igxp. Every assertion here is about TEXT: `toBeVisible` on a
+  // `dt` passes against an empty one, which is the whole failure this exists
+  // to catch — a page drawing the right NUMBER of blank rows.
+  test("? opens help, and its rows carry the keys and the words, not empty frames", async ({
+    page,
+  }) => {
+    await openHelp(page);
+
+    // The five pages are `help::pages()`; the page spells no title.
+    await expect(page.locator(".help-tab")).toHaveText([
+      "Welcome",
+      "Navigation",
+      "Editing",
+      "Panels & Views",
+      "Commands",
+    ]);
+
+    // Welcome is prose end to end: every row is a `dt` with words in it and
+    // NO `dd` beside it — the rule `paintHelp` encodes as "a prose row has no
+    // description and spans both columns". An empty `dd` per prose row would
+    // leave a blank grid cell, and nothing but a count can see that.
+    const welcome = await pairs(page);
+    expect(welcome.length).toBeGreaterThan(20);
+    expect(welcome.every((row) => row.description === null)).toBe(true);
+    expect(welcome.filter((row) => row.kind === "help-banner")).toEqual([
+      { kind: "help-banner", keys: "  improvise", description: null },
+    ]);
+    expect(welcome.filter((row) => row.kind === "help-heading").map((r) => r.keys)).toEqual([
+      "Quick start",
+      "Core concepts",
+    ]);
+    // A blank row is the ONE row that is legitimately empty, so the
+    // "nothing is blank" rule below has to be stated over the others.
+    expect(
+      welcome.some((row) => row.kind === "help-blank" && row.keys === ""),
+    ).toBe(true);
+    expect(
+      welcome.filter((row) => row.kind !== "help-blank").every((row) => row.keys.trim() !== ""),
+    ).toBe(true);
+
+    await page.keyboard.press("l");
+    await expect(page.locator(".help-tab.active")).toHaveText("Navigation");
+    const navigation = await pairs(page);
+
+    // The keys rows, which are the ones with a `dd`. `keys_for` drops a row
+    // whose command is unbound, so a row that is HERE claims a real binding.
+    const keyed = navigation.filter((row) => row.kind === "help-keys");
+    expect(keyed.length).toBeGreaterThan(10);
+    // The injection improvise-igxp names: blank `keys.textContent` in
+    // `paintHelp`. Every one of these is a string off the keymap.
+    expect(keyed.every((row) => row.keys.trim() !== "")).toBe(true);
+    expect(keyed.every((row) => (row.description ?? "").trim() !== "")).toBe(true);
+
+    // And the pairing itself, for three rows whose keys are the keymap's:
+    // `h j k l` and the arrows are four bindings compacted into one row, and
+    // a page that drew keys of its own would not know to.
+    const described = (text) => keyed.find((row) => row.description === text)?.keys;
+    expect(described("Move one cell")).toBe("h  j  k  l  ←  ↑  →  ↓");
+    expect(described("Next page-axis item")).toBe("]");
+    expect(described("Drill into selected cell")).toBe(">");
+
+    await page.keyboard.press("Escape");
+    await expect(page.locator("#help")).toBeHidden();
+    await expect(page.locator("#mode")).toHaveText("NORMAL");
+  });
+
+  // The assertions above are still about a string on a screen. This one asks
+  // whether the string is TRUE: the key the overlay advertises is read out of
+  // the DOM and then pressed, so a page drawing a plausible key it does not
+  // have — the exact failure a hand-spelled table decays into — is caught
+  // without this spec holding a second copy of the keymap.
+  test("a key the overlay advertises does what the overlay says it does", async ({
+    page,
+  }) => {
+    await openHelp(page);
+    await page.keyboard.press("l");
+    await expect(page.locator(".help-tab.active")).toHaveText("Navigation");
+
+    const rows = await pairs(page);
+    const advertised = rows.find((row) => row.description === "Jump to last row")?.keys;
+    // Not `toBeTruthy`: an empty string is falsy and would read as "the row
+    // is missing", which is a different fault with a different repair.
+    expect(advertised, "no row advertises jump-to-last-row").not.toBe(undefined);
+    expect(advertised.trim(), "the row is drawn, and drawn blank").not.toBe("");
+
+    await page.keyboard.press("Escape");
+    await expect(page.locator("#help")).toBeHidden();
+
+    const rows_ = page.locator("#grid tbody tr");
+    await expect(rows_.first().locator("td.selected")).toHaveCount(1);
+    await expect(rows_.last().locator("td.selected")).toHaveCount(0);
+
+    // One binding per row is the compaction (`h  j  k  l` is four), so press
+    // the first key the row names rather than the whole string.
+    await page.keyboard.press(advertised.trim().split(/\s+/)[0]);
+
+    await expect(rows_.last().locator("td.selected")).toHaveCount(1);
+    await expect(page.locator("#grid tbody td.selected")).toHaveCount(1);
+  });
+});