Merge pull request #149 from fiddlerwoaroof/fix/rr8n-one-hint-table

fix(tui): one key table on screen, and it is the derived one (improvise-rr8n)

context/code/tui.md

@@ -183,6 +183,43 @@ decoration", and which is which is local. Truncation goes through
 `ui::text::truncate` (improvise-8l0c), so an overflow is a `…` rather than a
 mid-token clip of `?:`.
 
+### One key table on screen, and it is the one derived from the keymap
+
+The TUI stacks the tile bar (row 3) directly above the status bar (row 4), and
+the import wizard's overlay sits above the status bar too, so a mode can put
+**two** key tables on screen at once. When it does, they disagree — different
+notation, different content, and in one case a flat contradiction.
+
+The rule is not "the overlay wins" or "the status bar wins". It is **keep
+whichever table is derived from the thing it describes** (improvise-rr8n):
+
+- **Tile select: the status bar wins.** `hint_line(&mode)` is read off the
+  keymap, so it cannot go stale, and it is a strict superset of what the tile
+  bar's literal carried — `Space`, `h`/`l` and `Tab` were only ever on the
+  derived line. The tile bar's own hints are therefore **deleted**, both of
+  them, and `TileBar::render` now advertises no keys at all. The one it drew in
+  Normal was false: it named `Ctrl+↑↓←→` as "to move tiles" while those four
+  keys are bound to `enter-tile-select`, which moves nothing.
+- **The import wizard: the overlay wins.** `hint_line(&AppMode::ImportWizard)`
+  is derived too, but from the *wrong input* — the wizard's real keys are
+  resolved per step inside `HandleWizardKey`, and one static line off the mode's
+  keymap is wrong on four of its eight steps, including telling the user `Esc`
+  cancels on `Done` where `Esc` goes back. So `draw_status` suppresses its hint
+  fallback for that one mode and lets the overlay's per-step hints stand alone.
+
+Two things worth keeping from how this survived. `every_hint_key_resolves_in_its_mode`
+checks `AppMode::hint_text()` against the keymaps and **cannot see a string
+literal in a widget**, which is the same blind spot improvise-wk18 names for
+`help.rs`. And the widget's own characterization test renders at 80 columns,
+where the hint does not fit and is never drawn — so the test that pins this
+widget's output was structurally incapable of showing either hint. A fixture
+width is part of what a test can see.
+
+The longer-term shape the ticket names is still open: the wizard's per-step keys
+belong in `advertised()` keyed by step, which would make them derived like
+everything else and let the status bar carry them correctly instead of being
+switched off.
+
 ### The caches are built here, per frame
 
 `project::formulas_of`, `category_tree_of`, `view_list_of`, `tile_bar_of`,

context/test-evidence/tui.md

@@ -730,3 +730,109 @@ prevent.
 
 Fault injected: the `Keeps::Left` arm rewritten to the body of the `Keeps::Right`
 arm. This test fails; the other four pass.
+
+---
+
+## `src/ui/tile_bar.rs` + `src/draw.rs` — one hint table on screen, and it is the derived one
+
+improvise-rr8n. Two hand-written key tables were drawn **beside** derived ones,
+in a different notation, and one of them was outright false.
+
+**The tile bar's non-TileSelect hint named keys that do something else.** It read
+`" Ctrl+↑↓←→ to move tiles"`, while `Ctrl-Left/Right/Up/Down` are bound in Normal
+to `enter-tile-select` — pressing one *enters* tile mode and moves nothing. It
+was also the only persistent on-screen documentation of those keys, and it spelt
+`Ctrl+` where `keymap::key_label`, and therefore every derived hint and help row,
+spells `Ctrl-`.
+
+**Nothing could have caught it.** `every_hint_key_resolves_in_its_mode` checks
+`AppMode::hint_text()` against the keymaps; a string literal in a widget is
+invisible to it. And the shipped fixture `renders_each_tile_with_its_axis_label`
+renders at **80 columns**, where the hint does not fit (`x + hint.width() <=
+area.x + area.width` is false) and is never drawn — so the one test that pins
+this widget's output could not see either hint at all. That is why the new tests
+render at 140.
+
+**The fix deletes rather than rewrites, and that departs from the ticket's AC.**
+improvise-rr8n proposed rewriting the string to `" Ctrl-↑↓←→ or T to pick a
+tile"` and suppressing the *status bar's* hint in TileSelect. Measured against
+the ticket's own second finding, that is backwards:
+
+```
+tile bar   (hand-written):  [Enter] cycle axis  [r/c/p] set axis  [←→] select  [Esc] cancel
+status bar (derived):       hl←→:select  Enter/Space:cycle  r:row  c:col  p:page  Esc/Tab:back
+```
+
+The derived line is a strict **superset** — it carries `Space`, `h`/`l` and
+`Tab`, which the tile bar's omits — and it cannot go stale, being read off the
+keymap. Suppressing it would have deleted information and kept the table that
+had just been shown to rot. So the tile bar now advertises nothing, and in
+Normal the derived line already says `T:tiles`; only the `Ctrl-`arrow spelling
+is lost, and it was never documented correctly.
+
+**The wizard goes the other way, and there the ticket is right.** Its overlay
+draws correct per-step hints, and the status bar drew one static
+`Space:toggle  c:cycle  Enter:next  Esc:cancel` under it — wrong on four of eight
+steps, including an outright contradiction on `Done`, where the bar says `Esc`
+cancels and `Esc` goes *back*. It is static because it is derived from the
+`ImportWizard` keymap, while the wizard's real keys are resolved per step inside
+`HandleWizardKey`; so the derivation is sound and its *input* is the wrong thing.
+`draw_status` therefore suppresses the hint fallback for that one mode.
+
+The asymmetry is the point: **keep whichever table is derived from the thing it
+describes.** For tile select that is the status line; for the wizard it is the
+overlay.
+
+### the_tile_bar_advertises_no_keys_of_its_own
+
+Renders the bar in Normal and TileSelect at 140 columns and requires that neither
+row names `Ctrl`, `Enter`, `Esc`, `←→` or `move tiles`. An absence assertion, so
+it is paired with the two below that require the information to still exist
+elsewhere.
+
+Two faults injected, both caught here and by nothing else:
+
+- The original `else` branch restored — fails in Normal on `"Ctrl"`, quoting the
+  whole false line.
+- Only the `else` branch deleted, keeping the TileSelect literal (the narrow
+  reading of finding 1) — fails in TileSelect on `"Enter"`. This is the rival
+  that matters, because it is the fix a reader of finding 1 alone would write,
+  and it leaves two disagreeing dialects stacked on adjacent rows.
+
+Note it would also fail against the ticket's own proposed replacement string,
+which still contains `Ctrl`. That is deliberate rather than accidental: the
+property asserted is "this widget names no keys", not "this widget's keys are
+currently right".
+
+### the_tile_bar_still_draws_its_tiles_at_every_width_it_used_to
+
+The counterweight. Deleting a `set_string` at the end of a render function is
+the kind of edit that takes a neighbour with it, so this pins the tiles and the
+`Tiles:` prefix at 80 and 140. It also covers the dead `x += 1` the deletion
+orphaned — clippy caught that one, but the test is what says the drawing still
+lands.
+
+### the_wizard_overlay_is_the_only_key_hint_on_screen
+
+The wizard half. Requires the bottom row in `ImportWizard` to name none of
+`Space:toggle`, `c:cycle`, `Esc:cancel`. Fails against the unfixed code quoting
+the full row, including the `Esc:cancel` that contradicts the overlay.
+
+### suppressing_the_wizard_hint_leaves_the_rest_of_the_bar
+
+The direction the suppression could overshoot: the mode label `IMPORT` and a real
+`status_msg` are not hints and must survive. A fix that blanked the bar in that
+mode, or that suppressed the *message* along with the hint, fails here.
+
+### the_status_bar_still_advertises_the_tile_keys
+
+The counterweight to the tile bar deletion, and the test that rejects the
+ticket's own AC: Normal must still say `T:tiles`, and TileSelect must still carry
+`Enter/Space:cycle`, `r:row` and `Esc/Tab:back`.
+
+Fault injected: suppressing the hint fallback unconditionally
+(`true => String::new()`) rather than for `ImportWizard` alone. All three wizard
+tests pass — including both of the ones *aimed* at the suppression — and only
+this one fails. That is the shape design-principles §7 names: the assertion next
+door is satisfied by the wrong fix, and the discriminating input is the mode the
+change was not supposed to touch.

src/draw.rs

@@ -454,6 +454,9 @@ fn draw_status(f: &mut Frame, area: Rect, app: &App) {
 
     let msg = match app.view_state.status_msg.is_empty() {
         false => app.view_state.status_msg.clone(),
+        // The wizard overlay draws its own per-step hints; one static table
+        // under it contradicts them on four of eight steps (improvise-rr8n).
+        true if matches!(app.view_state.mode, AppMode::ImportWizard) => String::new(),
         true => app.hint_text(),
     };
 
@@ -584,6 +587,61 @@ mod bottom_bar_tests {
             .collect()
     }
 
+    /// The wizard overlay draws correct per-step hints; the status bar stacked a
+    /// second static table under it (improvise-rr8n) — `context/test-evidence/tui.md`.
+    #[test]
+    fn the_wizard_overlay_is_the_only_key_hint_on_screen() {
+        let mut app = App::new(Workbook::new("T"), None);
+        app.view_state.mode = AppMode::ImportWizard;
+        let row = bottom_row(&app, 100);
+        for token in ["Space:toggle", "c:cycle", "Esc:cancel"] {
+            assert!(
+                !row.contains(token),
+                "the status bar names {token:?} under an overlay that contradicts \
+                 it on four of eight steps: {row:?}"
+            );
+        }
+    }
+
+    /// Suppressing the hint must not blank the bar: the mode label and any
+    /// status message still belong there. improvise-rr8n.
+    #[test]
+    fn suppressing_the_wizard_hint_leaves_the_rest_of_the_bar() {
+        let mut app = App::new(Workbook::new("T"), None);
+        app.view_state.mode = AppMode::ImportWizard;
+        assert!(
+            bottom_row(&app, 100).contains("IMPORT"),
+            "the mode label is not a hint and must survive"
+        );
+
+        app.view_state.status_msg = "no array found in that JSON".to_string();
+        assert!(
+            bottom_row(&app, 100).contains("no array found in that JSON"),
+            "a real status message must still reach the bar"
+        );
+    }
+
+    /// The counterweight to deleting the tile bar's own hint: the tile keys are
+    /// still advertised, by the derived line. improvise-rr8n.
+    #[test]
+    fn the_status_bar_still_advertises_the_tile_keys() {
+        let mut app = App::new(Workbook::new("T"), None);
+        assert!(
+            bottom_row(&app, 200).contains("T:tiles"),
+            "Normal must still say how to reach tile select"
+        );
+
+        app.view_state.mode = AppMode::TileSelect;
+        let row = bottom_row(&app, 200);
+        for token in ["Enter/Space:cycle", "r:row", "Esc/Tab:back"] {
+            assert!(
+                row.contains(token),
+                "TileSelect lost {token:?}, which the tile bar's own hint never \
+                 carried in full: {row:?}"
+            );
+        }
+    }
+
     fn app_with_minibuffer_and_message() -> App {
         let mut app = App::new(Workbook::new("T"), None);
         app.view_state.mode = AppMode::category_add();

src/ui/tile_bar.rs

@@ -126,21 +126,10 @@ impl<'a> Widget for TileBar<'a> {
         // Draw overflow indicator if tiles remain after the visible area
         if last_drawn + 1 < labels.len() && x < area.x + area.width {
             buf.set_string(x, area.y, "▶", Style::default().fg(Color::DarkGray));
-            x += 1;
         }
 
-        // Hint
-        if matches!(self.mode, AppMode::TileSelect) {
-            let hint = " [Enter] cycle axis  [r/c/p] set axis  [←→] select  [Esc] cancel";
-            if x + hint.width() as u16 <= area.x + area.width {
-                buf.set_string(x, area.y, hint, Style::default().fg(Color::DarkGray));
-            }
-        } else {
-            let hint = " Ctrl+↑↓←→ to move tiles";
-            if x + hint.width() as u16 <= area.x + area.width {
-                buf.set_string(x, area.y, hint, Style::default().fg(Color::DarkGray));
-            }
-        }
+        // The bar draws tiles and advertises no keys: the status line's hint is
+        // derived from the keymap and this one was not (improvise-rr8n).
     }
 }
 
@@ -218,4 +207,45 @@ mod tests {
             " Tiles:   [_Index ·]  [_Dim ·]  [_Measure Col]  [Region Row]  [Product Col]"
         );
     }
+
+    /// The tile bar advertised keys that do something else (improvise-rr8n) —
+    /// `context/test-evidence/tui.md`.
+    #[test]
+    fn the_tile_bar_advertises_no_keys_of_its_own() {
+        let app = app_with_two_categories();
+        let cache = project::tile_bar_of(&app);
+        for mode in [AppMode::Normal, AppMode::TileSelect] {
+            // Wide enough that a hint would fit: the shipped 80-column fixture
+            // is too narrow to draw one, which is how the false claim survived.
+            let area = Rect::new(0, 0, 140, 1);
+            let mut buf = Buffer::empty(area);
+            TileBar::new(&cache, &mode, 0).render(area, &mut buf);
+            let text = buf_text(&buf);
+            for token in ["Ctrl", "Enter", "Esc", "←→", "move tiles"] {
+                assert!(
+                    !text.contains(token),
+                    "the tile bar names {token:?} in {mode:?}, and nothing checks \
+                     that claim against the keymap: {text:?}"
+                );
+            }
+        }
+    }
+
+    /// The tiles themselves are what the bar is for, and deleting the hint must
+    /// not delete them. improvise-rr8n.
+    #[test]
+    fn the_tile_bar_still_draws_its_tiles_at_every_width_it_used_to() {
+        let app = app_with_two_categories();
+        let cache = project::tile_bar_of(&app);
+        for width in [80u16, 140] {
+            let area = Rect::new(0, 0, width, 1);
+            let mut buf = Buffer::empty(area);
+            TileBar::new(&cache, &AppMode::TileSelect, 0).render(area, &mut buf);
+            let text = buf_text(&buf);
+            assert!(
+                text.contains("[Region Row]") && text.contains("Tiles:"),
+                "at width {width} the bar lost its tiles: {text:?}"
+            );
+        }
+    }
 }