[ok] explain <Table> is uninformatively terse; reconsider whether the [ok] line duplicates the SQL-echo line above it #9

Closed
opened 2026-05-28 14:44:32 +01:00 by oliversturm · 1 comment
oliversturm commented 2026-05-28 14:44:32 +01:00 (Migrated from github.com)

Observed

For explain show data Customers, the output panel renders:

  1. The plan-SQL echo (uncoloured) — from render_explain_plan (src/output_render.rs:250)
  2. The annotated plan tree
  3. [ok] explain Customers — from note_ok_summary (src/app.rs:1429), using command.display_subject(), which for Explain { … } falls through to just the target table name (src/dsl/command.rs:981)

So the user sees "Customers" twice — once in the SQL echo, once in the [ok] line — and no clear indication of what succeeded.

Two threads

1. Bug — make [ok] explain ... informative.
At minimum echo the wrapped query verb + subject (e.g. [ok] explain show data Customers). Better: include something that distinguishes it from a non-explain run.

2. Design question — is the [ok] summary line carrying its weight?
TUI screen space is limited. The "running …" / SQL-echo line and the [ok] line together don't say much more than either of them alone. Alternatives worth considering:

  • A status icon (green ✓ / red ✗) appended to the input-echo line on completion, removing the separate [ok] line.
  • Keep [ok] only when there's actual additional content to convey (e.g. row counts on writes); suppress it for pure-render commands like explain and show data.

This second thread needs a deliberate call, not a quick fix — possibly an ADR amendment if it touches ADR-0014 / ADR-0028 output conventions.

### Observed For `explain show data Customers`, the output panel renders: 1. The plan-SQL echo (uncoloured) — from `render_explain_plan` (`src/output_render.rs:250`) 2. The annotated plan tree 3. `[ok] explain Customers` — from `note_ok_summary` (`src/app.rs:1429`), using `command.display_subject()`, which for `Explain { … }` falls through to just the target table name (`src/dsl/command.rs:981`) So the user sees "Customers" twice — once in the SQL echo, once in the [ok] line — and no clear indication of *what* succeeded. ### Two threads **1. Bug — make `[ok] explain ...` informative.** At minimum echo the wrapped query verb + subject (e.g. `[ok] explain show data Customers`). Better: include something that distinguishes it from a non-explain run. **2. Design question — is the `[ok]` summary line carrying its weight?** TUI screen space is limited. The "running …" / SQL-echo line and the `[ok]` line together don't say much more than either of them alone. Alternatives worth considering: - A status icon (green ✓ / red ✗) appended to the input-echo line on completion, removing the separate `[ok]` line. - Keep `[ok]` only when there's actual additional content to convey (e.g. row counts on writes); suppress it for pure-render commands like `explain` and `show data`. This second thread needs a deliberate call, not a quick fix — possibly an ADR amendment if it touches ADR-0014 / ADR-0028 output conventions.
oliversturm commented 2026-05-30 22:39:01 +01:00 (Migrated from github.com)

Resolved in 8311de4 (ADR-0040).

Started from #9's narrow complaint ([ok] explain is terse, and post-ADR-0039 explain select even rendered [ok] explain with an empty subject) and generalised via an audit of the whole command surface: the [ok] <verb> <subject> summary line duplicated the echo line directly above it on every command — its only unique contribution was the success-vs-error signal.

Decision (ADR-0040): retire the [ok] summary line (and the symmetric "<verb> <subject>" failed: prefix). A command's echo line now resolves from running: <input> to <input> ✓ (success) / <input> ✗ (runtime failure) on completion — the marker carries the one signal that wasn't already on screen, and saves a row on a small TUI. The failure reason still renders beneath ([error]-tagged); content lines (row counts, structure, data, plan tree, teaching echo) are unchanged.

Scope: executed commands (success ✓ / runtime-failure ✗ / skip ✓ / replay). Parse-time and pre-flight rejections never run and keep their running: + caret rendering (their caret alignment is pinned to that prefix). App-command [ok] lines (rebuild/export/now editing/replay — N) are payload-bearing, have no echo to mark, and stay as-is.

Mechanics: echo lines carry an EchoStatus; executed commands push Pending and the result event resolves the oldest pending echo (the db worker is FIFO, so this is correct even if commands interleave, and never strands an earlier echo on running:).

Both threads of the issue addressed (informative success signal + the [ok] line's value reconsidered). Design and implementation each /runda'd; live-app verified (success ✓, runtime failure ✗ + reason, drop table if exists … ✓, create … if not exists … ✓ + skip note, parse error keeps running: + caret, app commands unaffected). 2089 tests / 0 / 1, clippy clean.

Resolved in 8311de4 (ADR-0040). Started from #9's narrow complaint (`[ok] explain` is terse, and post-ADR-0039 `explain select` even rendered `[ok] explain` with an empty subject) and generalised via an audit of the whole command surface: the `[ok] <verb> <subject>` summary line **duplicated** the echo line directly above it on every command — its only unique contribution was the success-vs-error signal. **Decision (ADR-0040):** retire the `[ok]` summary line (and the symmetric `"<verb> <subject>" failed:` prefix). A command's echo line now resolves from `running: <input>` to `<input> ✓` (success) / `<input> ✗` (runtime failure) on completion — the marker carries the one signal that wasn't already on screen, and saves a row on a small TUI. The failure reason still renders beneath (`[error]`-tagged); content lines (row counts, structure, data, plan tree, teaching echo) are unchanged. **Scope:** executed commands (success ✓ / runtime-failure ✗ / skip ✓ / replay). Parse-time and pre-flight rejections never run and keep their `running:` + caret rendering (their caret alignment is pinned to that prefix). App-command `[ok]` lines (`rebuild`/`export`/`now editing`/`replay — N`) are payload-bearing, have no echo to mark, and stay as-is. **Mechanics:** echo lines carry an `EchoStatus`; executed commands push `Pending` and the result event resolves the **oldest** pending echo (the db worker is FIFO, so this is correct even if commands interleave, and never strands an earlier echo on `running:`). Both threads of the issue addressed (informative success signal + the `[ok]` line's value reconsidered). Design and implementation each `/runda`'d; live-app verified (success ✓, runtime failure ✗ + reason, `drop table if exists … ✓`, `create … if not exists … ✓` + skip note, parse error keeps `running:` + caret, app commands unaffected). 2089 tests / 0 / 1, clippy clean.
Sign in to join this conversation.