8311de44a8
An audit of the command surface found the `[ok] <verb> <subject>` summary line duplicated the echo line above it everywhere; its only unique signal was success-vs-error. Retire it: a command's echo line now resolves from `running: <input>` to `<input> ✓` / `<input> ✗` on completion, and the symmetric `"<verb> <subject>" failed:` prefix is dropped (only the reason remains). Content lines (row counts, structure, plan tree, teaching echo) are unchanged. Echo lines carry an EchoStatus; executed commands push Pending and resolve the oldest-pending echo on their result event (FIFO worker — correct under interleaving). Parse-time and pre-flight rejections are not executed and keep their running: + caret rendering. App-command [ok] lines (rebuild/export/replay) are payload-bearing and untouched. ADR-0040.
185 lines
8.2 KiB
Markdown
185 lines
8.2 KiB
Markdown
# ADR-0040: A per-command completion marker (✓/✗) replaces the `[ok]` summary line
|
|
|
|
## Status
|
|
|
|
**Accepted** — 2026-05-30 (issue #9). Amends the output conventions of
|
|
ADR-0014 (data operations), ADR-0028 (query plans / `explain`), and
|
|
ADR-0019 (failure rendering); builds on ADR-0037's mode-tagged echo
|
|
line.
|
|
|
|
## Context
|
|
|
|
Every DSL / data / SQL command renders its outcome in three parts:
|
|
|
|
```
|
|
[<mode>] running: <full input> ← the echo line (OutputKind::Echo, pushed at submit)
|
|
[ok] <verb> <subject> ← the summary line (note_ok_summary)
|
|
<result content> ← structure / data table / plan tree / row counts
|
|
```
|
|
|
|
Issue #9 started narrow — `[ok] explain Customers` is uninformatively
|
|
terse, and (after ADR-0039 routed SQL through the same path)
|
|
`explain select …` even renders `[ok] explain` with an **empty**
|
|
subject. Pulling the thread, the question generalised: *what does the
|
|
`[ok]` line add at all?*
|
|
|
|
An audit of the whole command surface (the two things the line
|
|
carries, against the rest of the output):
|
|
|
|
| What `[ok]` carries | Already present? |
|
|
|---|---|
|
|
| verb + subject | **Yes** — verbatim, and more fully, in the echo line directly above |
|
|
| success signal (vs `[error]`) | **No** — its only unique contribution |
|
|
|
|
By command shape:
|
|
|
|
- **Content-bearing** (create table, add/drop/change column,
|
|
insert/update/delete, show data/table, explain): the content below
|
|
*is* the proof of success; the verb+subject duplicates the echo.
|
|
- **Content-less** (drop table/index/relationship, add index,
|
|
add/drop constraint): `[ok]` is the only line printed — but what
|
|
does the work there is the **success signal**, not the verb+subject.
|
|
|
|
The failure path is symmetric: `dsl.failed` =
|
|
`'"{verb} {subject}" failed: {rendered}'` — the same redundant
|
|
verb+subject prefix, plus the genuinely useful rendered reason.
|
|
|
|
**Conclusion:** the summary line's text is redundant everywhere; its
|
|
sole irreplaceable job is the success-vs-error distinction, needed on
|
|
every command. So it can be removed entirely once that one signal
|
|
moves elsewhere.
|
|
|
|
## Decision
|
|
|
|
Replace the `[ok]` / `"…" failed:` **summary lines** with an inline
|
|
**completion marker** on the echo line:
|
|
|
|
- **Success:** the echo line gains a trailing green **✓**; the
|
|
`[ok] <verb> <subject>` line is no longer emitted.
|
|
- **Failure:** the echo line gains a trailing red **✗**; the
|
|
redundant `"<verb> <subject>" failed:` prefix is dropped and only
|
|
the rendered reason is shown (still `[error]`-tagged).
|
|
- **The "running:" prefix is a pending state.** The echo reads
|
|
`running: <input>` while the worker is in flight and becomes
|
|
`<input> ✓` / `<input> ✗` on completion — the line carries the raw
|
|
input plus an outcome status the renderer decorates.
|
|
- **Marker placement: inline**, immediately after the echoed input
|
|
(`[advanced] drop table Orders ✓`). Robust to long inputs that wrap
|
|
and needs no render-time width arithmetic, unlike a right-aligned
|
|
column.
|
|
- **Content is unchanged.** Row-count footers, structure renders, data
|
|
tables, plan trees, cascade summaries, auto-fill notes, and the
|
|
ADR-0038 teaching echo all still render — they carry the real
|
|
information.
|
|
|
|
### Scope
|
|
|
|
**Executed commands only.** The marker covers commands that reach the
|
|
worker: success (✓), runtime failure (✗), a skipped no-op (✓), and
|
|
`replay` (✓/✗ on completion). **Parse-time and pre-flight rejections**
|
|
(a parse error, the form-B positional pre-flight, the wrong-count DSL
|
|
insert) never reach the worker and do *not* use the `[ok]`/`failed:`
|
|
summary — they keep their existing rendering unchanged
|
|
(`running: <input>` + a `^` caret + a `parse error:`/teaching
|
|
message). They are "didn't run," not "ran and failed," and their caret
|
|
alignment is pinned to the `running: ` prefix, so they stay as-is.
|
|
|
|
This applies to the **DSL / data / SQL command family** — the
|
|
commands that push an `OutputKind::Echo` line and route success
|
|
through `note_ok_summary`. App-command successes
|
|
(`[ok] rebuild — …`, `[ok] export — wrote …`, `[ok] now editing: …`)
|
|
are **out of scope and unchanged**: they have no echo line to mark,
|
|
do not go through `note_ok_summary`, and their `[ok]` text carries a
|
|
real payload (path, summary, project name) rather than a redundant
|
|
verb+subject. A small residual inconsistency (app commands keep an
|
|
`[ok]` line; DSL commands use a marker) is accepted as the lesser
|
|
evil — converting payload-bearing lines to a bare marker would *lose*
|
|
information, the opposite of this change's intent.
|
|
|
|
## Example
|
|
|
|
Before:
|
|
```
|
|
[advanced] running: explain show data Customers
|
|
[ok] explain Customers
|
|
SELECT "id", "Name", "Age" FROM "Customers"
|
|
└─ SCAN Customers
|
|
```
|
|
|
|
After:
|
|
```
|
|
[advanced] explain show data Customers ✓
|
|
SELECT "id", "Name", "Age" FROM "Customers"
|
|
└─ SCAN Customers
|
|
```
|
|
|
|
Failure, after:
|
|
```
|
|
[advanced] insert into Customers values (1, 'x') ✗
|
|
error: NOT NULL constraint failed: Customers.Age
|
|
```
|
|
|
|
## Consequences
|
|
|
|
- **Saves one row per command** on a space-constrained TUI, and the
|
|
surviving signal (✓/✗) is sharper than a prose `[ok]` line.
|
|
- **The echo line gains a lifecycle** (pending → ok/err). The status
|
|
is set on completion by locating the line — the existing
|
|
`rfind(|l| l.kind == OutputKind::Echo)` already does this lookup.
|
|
- **Snapshot churn:** every test that captures command output is
|
|
re-baselined. The marker is part of the captured frame, so the
|
|
diffs are mechanical but broad.
|
|
- **i18n:** `ok.summary` is retired; `dsl.failed` is reduced to the
|
|
rendered reason. The marker glyphs are not translatable strings
|
|
(they are symbols), consistent with the existing tree connectors.
|
|
|
|
## Implementation notes (from the design `/runda`)
|
|
|
|
- **Echo line carries an outcome status** — `Pending` / `Ok` / `Err` —
|
|
on the `OutputKind::Echo` line. The renderer decorates: `Pending` →
|
|
`running: <input>`; `Ok` → `<input> ✓` (green); `Err` → `<input> ✗`
|
|
(red). The raw input is stored; "running:" and the marker are
|
|
render-time decoration keyed on status.
|
|
- **Attribution: mark the *oldest* still-`Pending` echo** when a
|
|
result event arrives — not merely the last echo line. `spawn_dsl_
|
|
dispatch` is fire-and-forget and input is not gated, so two commands
|
|
*can* be in flight; results arrive in submission order (the db
|
|
worker is FIFO), so the oldest-pending echo is the correct target
|
|
and a finished command can never leave an earlier one stuck on
|
|
`running:`. In practice execution is sub-millisecond and a human
|
|
cannot interleave submissions, so there is effectively one command
|
|
in flight; the oldest-pending rule is the cheap, order-correct
|
|
formalisation of that. (A fully order-independent echo-id round-trip
|
|
was considered and rejected as disproportionate plumbing.)
|
|
- **Synchronous error paths set `Err` at push time.** Parse errors,
|
|
the form-B positional pre-flight, and the wrong-count DSL-insert
|
|
pre-flight (`dispatch_dsl`) push their echo already-`Err` (the
|
|
outcome is known immediately, no worker round-trip), then render the
|
|
reason — so they never dangle on `Pending`.
|
|
- **Skipped no-ops set `Ok`.** `CREATE/DROP IF [NOT] EXISTS` that
|
|
no-op (`DslCreateSkipped` / `DslDropSkipped` /
|
|
`DslDropIndexSkipped` / `DslCreateIndexSkipped`) mark their echo ✓
|
|
(the command succeeded as a no-op) and keep their explanatory skip
|
|
note beneath.
|
|
- **`note_ok_summary` stops emitting the `[ok]` line** but stays as
|
|
the success hook: it sets the echo status to `Ok` and still consumes
|
|
the ADR-0038 `pending_echo` teaching lines.
|
|
- **i18n:** `ok.summary` is retired; `dsl.failed` becomes just the
|
|
rendered reason (no `"{verb} {subject}" failed:` prefix). The
|
|
`replay.completed`, `rebuild_ok`, `export_ok`, `switched_ok` `[ok]`
|
|
templates are payload-bearing and untouched.
|
|
|
|
## Out of scope
|
|
|
|
- App-command `[ok]` lines (see Scope).
|
|
- The `[WRN]` validity indicator (typing-time; ADR-0027) and the
|
|
`[error]`/`[system]` **tag colours** (issue #10) — orthogonal.
|
|
|
|
## See also
|
|
|
|
- ADR-0014 — data operations, auto-show, the summary/row-count footers.
|
|
- ADR-0028 — `explain` output (SQL echo + plan tree) this sits above.
|
|
- ADR-0037 — the mode-tagged echo line the marker attaches to.
|
|
- ADR-0019 — the friendly-error rendering the failure reason flows through.
|
|
- Issue #9 — the report and the audit that generalised it.
|