diff --git a/docs/adr/0040-completion-marker-replaces-ok-summary.md b/docs/adr/0040-completion-marker-replaces-ok-summary.md new file mode 100644 index 0000000..bc7cd3a --- /dev/null +++ b/docs/adr/0040-completion-marker-replaces-ok-summary.md @@ -0,0 +1,184 @@ +# 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: + +``` +[] running: ← the echo line (OutputKind::Echo, pushed at submit) +[ok] ← the summary line (note_ok_summary) + ← 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] ` line is no longer emitted. +- **Failure:** the echo line gains a trailing red **✗**; the + redundant `" " 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: ` while the worker is in flight and becomes + ` ✓` / ` ✗` 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: ` + 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: `; `Ok` → ` ✓` (green); `Err` → ` ✗` + (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. diff --git a/docs/adr/README.md b/docs/adr/README.md index 581ade8..9a1b07c 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -45,3 +45,4 @@ This directory contains the project's ADRs, recorded per - [ADR-0037 — Execution-time mode side-channel (the three-way submission mode)](0037-execution-time-mode-side-channel.md) — **Accepted** (design agreed 2026-05-27; channel **implemented + verified end-to-end** by its motivating consumer — ADR-0038's fully-shipped DSL → SQL teaching echo — across handoff-46 `04c8e42` (channel + first echo slice), handoff-47 `90479cb` (full Bucket A), `275c726` (Bucket B resolved-name + multi-statement renderers), `e6ad1ae` (the category-3 `--dont-convert` caveat — gated on this channel too), and `2aab457` (the §4 styled-runs rendering polish)), **redeems the follow-up deferred by ADR-0033 Amendment 3** (which named this ADR and its motivating consumer). Establishes the channel that lets a command know, **at execution time**, the effective mode it ran under — so execution can adjust **output** without touching **identity** (the motivating case: a DSL-form `create table` echoing the equivalent SQL when run in advanced mode, silent in simple — ADR-0030 §10, realised by ADR-0038). Introduces a **new per-submission enum `SubmissionMode` { Simple, Advanced, AdvancedOneShot }** — *refining* Amendment 3's "widen `Mode`" sketch: the persistent input `Mode` stays **two-way** (`mode.rs` keeps the one-shot `:` out of persistent state), and the three-way distinction lives on the per-submission channel where the transient `:` belongs. Resolved at submit time (Simple+`:` → `AdvancedOneShot`; Advanced `:` is a no-op), threaded through `Action::ExecuteDsl` → worker, **output-only** (no executor branches its *effect* on it — Amendment 3 forbids behavioural mode dependence). The worker builds the teaching echo (+ category-3 expansion data — ADR-0038) for DSL-form commands in advanced/one-shot mode and returns it; the App renders it beneath `[ok]`. Co-located with execution because the echo's harder forms (resolved auto-names, generated `shortid`s, conversion counts) are worker-computed facts, and gating on mode means the work happens only when shown. Alternatives weighed + rejected: widening `Mode` (conflates transient/persistent state); App-side gating with the worker always emitting echo data (computes unconditionally, doesn't generalise, re-opens the render-side framing ruled against). Scope: channel + resolution rule only — the renderer/catalogue/`Value → SQL-literal` are ADR-0038, the `ALTER COLUMN` gap-fill is the ADR-0035 amendment - [ADR-0038 — The DSL → SQL teaching echo](0038-dsl-to-sql-teaching-echo.md) — **Accepted** (design agreed 2026-05-27; **fully implemented + verified** — every catalogue row in §7 Buckets A + B and the §6 category-3 prose round-trips per line through the advanced walker per §1, and the §4 de-emphasised styled-runs polish is wired: handoff-46 `04c8e42` shipped the channel + create-table slice, handoff-47 `90479cb` the full Bucket A expansion + a skeleton contract-gap fix (dropped per-column `DEFAULT`/`CHECK`), `275c726` the Bucket B resolved-name + multi-statement renderers (auto- and user-named `add index`, positional `drop index`, `add`/`drop relationship` in both selector forms, `drop column --cascade`, `add relationship --create-fk`), `e6ad1ae` the last category-3 line — the `change column --dont-convert` *caveat* (shortid + transform notes were already surfaced via pre-existing `client_side.*` keys), and `2aab457` the §4 styled-runs polish: a new `OutputKind::TeachingEcho` custom rendering branch (dimmed `Executing SQL:` prefix + the SQL re-lexed in advanced mode for token-class colouring, same as the input echo) plus a new `OutputStyleClass::Hint` for every cat-3 prose line — caveat *and* the existing illuminating notes, user-confirmed broader scope), **realises ADR-0030 §10** (the teaching bridge) — the Phase-5 echo **ADR-0035 §12 forward-referenced** — building on **ADR-0037** (the `SubmissionMode` gate) and **ADR-0035 Amendment 2** (standard-first dialect + `ALTER COLUMN` gap-fill). When a **DSL-form** command runs in advanced/one-shot mode, the worker emits the equivalent SQL beneath `[ok]` as a de-emphasised styled `OutputLine` (ADR-0028); the App renders it. **Defining invariant — the copy-paste contract:** every echoed line is *runnable advanced-mode SQL* (round-trip-tested: parse the echo → same-effect command; a planned "copy the echo" affordance depends on it). **Type vocabulary = the playground's own keywords** (`serial`/`shortid`/…, accepted by `from_sql_name`, decision (a)); **statement shape = the standard-first dialect** (Am2). **DML uses substituted literals, not `?`** (per-type `Value → SQL-literal`, round-trip-safe; `blob` moot — no literal syntax exists; auto-gen columns omitted to match `do_insert` + X4). **Firing reality — a DDL + `show data` feature:** in advanced mode `insert`/`update`/`delete … where` are SQL-first (`Sql*` = already SQL = nothing to echo per §10); only DSL-*only* spellings echo (DDL + `show data` + the `delete`/`update … --all-rows` fall-throughs — the latter via **ADR-0033 Amendment 4**, a bug-fix folded in here that reverses Amendment 3's `update … --all-rows` misparse). **Three-category framework** for "what happens beyond the literal SQL": **(1) engine-implementation-hiding** (the rebuild, rowid PK, non-PK `serial` MAX+1) — *never surfaced*; **(2) decomposable into advanced SQL** (`drop column --cascade`, `--create-fk` relationship) — *shown as the runnable multi-line sequence, one statement per line*; **(3) playground type-behaviour with no SQL-expressible form** (`shortid` generation — no `shortid()`; type-conversion transforms — no `USING`) — *de-emphasised prose expansion from the worker's `client_side.*` notes*. Carries the **full catalogue** (Buckets A single-statement / B resolved-name + multi-line / C no-echo) mapping every DSL-form command to its echo. OOS: reverse SQL→DSL echo (§13 OOS-5), app commands / `show table` / `explain` / `replay`, a `blob` literal, the column-level UNIQUE/CHECK drop residual (Bucket C until Am2's gap closes), and surfacing any category-1 engine internal - [ADR-0039 — EXPLAIN over advanced-mode SQL queries](0039-explain-over-advanced-sql.md) — **Accepted** (2026-05-27), **implemented 2026-05-30 (issue #7)**, **supersedes ADR-0030 §13 OOS-2**. Lets `explain` wrap the advanced SQL commands (`Select`/`SqlInsert`/`SqlUpdate`/`SqlDelete`, plus `with`/CTE which builds a `Select`) in addition to the DSL `ShowData`/`Update`/`Delete` it already covers (ADR-0028), running `EXPLAIN QUERY PLAN` over the validated SQL text through the existing ADR-0028 span-styled plan tree (advanced mode only; DSL `explain` unchanged in both modes). Implemented via a second `Advanced` `explain` CommandNode (`EXPLAIN_SQL`) registered under the shared `explain` entry word — reusing the established `insert`/`update`/`delete` shared-word dispatch (`decide`: SQL-first / DSL-fallback), so `explain show data …` and DSL-only `--all-rows` still reach the DSL node; rejected a `DynamicSubgrammar` mode-gate (its resolution cache key omits `mode`). `build_explain_sql` slices the inner SQL off the source (excludes `explain`) and reuses the existing SQL builders; `do_explain_plan` runs the carried text verbatim, no params. Advanced `explain update`/`delete` now route through SQL (identical plan, full SQL syntax); DSL-explain tests pinned to simple mode. Reframed OOS-2 as a *deferred* exclusion (per ADR-0000's out-of-scope discipline), not a rejection. OOS (deferred): EXPLAIN of DDL (no query plan exists) +- [ADR-0040 — A per-command completion marker (✓/✗) replaces the `[ok]` summary line](0040-completion-marker-replaces-ok-summary.md) — **Accepted 2026-05-30 (issue #9)**, amends ADR-0014 / ADR-0028 / ADR-0019 output conventions, builds on ADR-0037's mode-tagged echo. An audit of the whole command surface found the `[ok] ` summary line duplicates the echo line above it (verb+subject) everywhere; its only unique contribution is the success-vs-error signal (and `explain select` even rendered `[ok] explain` with an empty subject post-ADR-0039). Decision: drop the `[ok]` line and the symmetric `"…" failed:` prefix; the echo line gains a trailing inline **✓** (green, success) / **✗** (red, failure) — `running:` becomes a pending state that resolves to ` ✓/✗` on completion (status set via the existing `rfind(Echo)` lookup). Content (row counts, structure, data, plan tree, teaching echo) unchanged. Scoped to the DSL/data/SQL family that has the redundant echo+`[ok]` pair; app-command `[ok]` lines (`rebuild`/`export`/`now editing`) are payload-bearing, have no echo to mark, and stay as-is. `ok.summary` retired; `dsl.failed` reduced to the rendered reason. Broad but mechanical snapshot churn. OOS: app-command `[ok]` lines, the `[WRN]` validity indicator, and the tag colours (issue #10) diff --git a/docs/requirements.md b/docs/requirements.md index 4da2876..52f49fe 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -135,8 +135,10 @@ handoff-14 cleanup; 449 after B2/C2.) belongs, not in persistent state). The runtime gates the ADR-0030 §10 teaching echo on it: a DSL-form command run in advanced/one-shot mode renders the equivalent advanced-mode SQL - beneath `[ok]`; simple-mode and SQL-entered submissions stay - silent. Echo coverage: **ADR-0038 is feature-complete** — every + beneath the command's echo line (the `[ok]` summary it once sat + under was retired by ADR-0040, issue #9 — the echo line now carries + a ✓/✗ completion marker instead); simple-mode and SQL-entered + submissions stay silent. Echo coverage: **ADR-0038 is feature-complete** — every catalogue row in §7 round-trips per line through the advanced walker (the §1 copy-paste contract; §6 category 2 holds it per line), every §6 category-3 line surfaces, and the §4 diff --git a/src/app.rs b/src/app.rs index 49fb9c3..9e046c1 100644 --- a/src/app.rs +++ b/src/app.rs @@ -9,7 +9,7 @@ use std::collections::VecDeque; use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyModifiers}; -use tracing::{trace, warn}; +use tracing::{debug, trace, warn}; use crate::action::Action; use crate::db::{ @@ -37,6 +37,21 @@ pub enum OutputKind { TeachingEcho, } +/// Completion state of an `OutputKind::Echo` line (ADR-0040). +/// +/// An echo for an *executed* command is pushed `Pending` (rendered +/// `running: `) and resolves to `Ok`/`Err` when the result +/// arrives — rendered ` ✓` / ` ✗`, replacing the old +/// `[ok]`/`failed:` summary line. Parse-time and pre-flight +/// rejections are not executed and carry `None` (they keep the +/// `running:` + caret rendering); non-echo lines also carry `None`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EchoStatus { + Pending, + Ok, + Err, +} + /// The semantic style class of an [`OutputSpan`] (ADR-0028 §5). /// /// A general output-styling vocabulary, resolved to a concrete @@ -82,6 +97,10 @@ pub struct OutputLine { /// these runs; when `None` it falls back to whole-line /// styling by `kind`. pub styled_runs: Option>, + /// Echo completion state (ADR-0040). `Some(_)` only on + /// `OutputKind::Echo` lines for executed commands; `None` + /// everywhere else (non-echo lines, parse/pre-flight echoes). + pub status: Option, } impl OutputLine { @@ -99,6 +118,20 @@ impl OutputLine { kind, mode_at_submission, styled_runs: Some(runs), + status: None, + } + } + + /// A `running: ` echo for an executed command, pushed + /// `Pending` and resolved to `Ok`/`Err` on completion (ADR-0040). + #[must_use] + pub fn echo(input: &str, mode: Mode) -> Self { + Self { + text: crate::t!("dsl.running", input = input), + kind: OutputKind::Echo, + mode_at_submission: mode, + styled_runs: None, + status: Some(EchoStatus::Pending), } } } @@ -470,8 +503,9 @@ impl App { description, } => { // No-op (CREATE TABLE IF NOT EXISTS on an existing - // table, ADR-0035 §4): the skip note, then the existing - // structure — no misleading "[ok] create table" line. + // table, ADR-0035 §4): a successful no-op — mark the + // echo ✓ (ADR-0040), then the skip note + structure. + self.mark_oldest_pending_echo(EchoStatus::Ok); self.note_system(crate::t!( "ddl.create_skipped_exists", name = command.target_table() @@ -484,8 +518,8 @@ impl App { } AppEvent::DslDropSkipped { command } => { // No-op (DROP TABLE IF EXISTS on an absent table, - // ADR-0035 §4, 4c): just the skip note — no structure, - // no misleading "[ok] drop table" line. + // ADR-0035 §4, 4c): successful no-op — echo ✓ + skip note. + self.mark_oldest_pending_echo(EchoStatus::Ok); self.note_system(crate::t!( "ddl.drop_skipped_absent", name = command.target_table() @@ -494,8 +528,9 @@ impl App { } AppEvent::DslDropIndexSkipped { command } => { // No-op (DROP INDEX IF EXISTS on an absent index, - // ADR-0035 §4d): just the skip note. `target_table()` - // returns the index name for `SqlDropIndex`. + // ADR-0035 §4d): successful no-op — echo ✓ + skip note. + // `target_table()` returns the index name for `SqlDropIndex`. + self.mark_oldest_pending_echo(EchoStatus::Ok); self.note_system(crate::t!( "ddl.drop_index_skipped_absent", name = command.target_table() @@ -504,9 +539,10 @@ impl App { } AppEvent::DslCreateIndexSkipped { command: _, name } => { // No-op (CREATE INDEX IF NOT EXISTS on an existing index - // name, ADR-0035 §4d): the skip note carries the resolved - // index name (the unnamed form's auto-name isn't on the - // command). No structure shown. + // name, ADR-0035 §4d): successful no-op — echo ✓ + skip + // note (the resolved index name; unnamed form's auto-name + // isn't on the command). + self.mark_oldest_pending_echo(EchoStatus::Ok); self.note_system(crate::t!("ddl.create_index_skipped_exists", name = name)); Vec::new() } @@ -608,6 +644,10 @@ impl App { path, message, } => { + // ADR-0040: if a command's persistence failed fatally, + // resolve its (pending) echo to ✗ before the quit banner, + // so the dying session doesn't leave a `running:` line. + self.mark_oldest_pending_echo(EchoStatus::Err); let banner = crate::t!( "fatal.persistence", operation = operation, @@ -728,6 +768,10 @@ impl App { count, warnings, } => { + // ADR-0040: the `replay` echo resolves ✓; the + // `[ok] replay — N command(s)` summary is payload-bearing + // (the count) and stays. + self.mark_oldest_pending_echo(EchoStatus::Ok); self.note_system(crate::t!( "replay.completed", path = path, @@ -747,6 +791,8 @@ impl App { command, error, } => { + // ADR-0040: the `replay` echo resolves ✗. + self.mark_oldest_pending_echo(EchoStatus::Err); // line_number == 0 is the runtime's signal that // file-open itself failed (no per-line context to // surface). Otherwise we lead with the line-number @@ -1331,12 +1377,7 @@ impl App { // hand it off as a dedicated `Action::Replay`, // keeping the worker out of the loop and the // history.log clean. - self.push_output(OutputLine { - text: crate::t!("dsl.running", input = input), - kind: OutputKind::Echo, - mode_at_submission: mode, - styled_runs: None, - }); + self.push_output(OutputLine::echo(input, mode)); vec![Action::Replay { path }] } Ok(cmd) => { @@ -1350,11 +1391,14 @@ impl App { &cmd, &self.schema_cache, ) { + // Pre-flight rejection (not executed): plain + // `running:` echo, `status: None` (ADR-0040 scope). self.push_output(OutputLine { text: crate::t!("dsl.running", input = input), kind: OutputKind::Echo, mode_at_submission: mode, styled_runs: None, + status: None, }); self.note_error(note); return vec![Action::JournalFailure { @@ -1371,11 +1415,14 @@ impl App { &cmd, &self.schema_cache, ) { + // Pre-flight rejection (not executed): plain + // `running:` echo, `status: None` (ADR-0040 scope). self.push_output(OutputLine { text: crate::t!("dsl.running", input = input), kind: OutputKind::Echo, mode_at_submission: mode, styled_runs: None, + status: None, }); for note in notes { self.note_error(note); @@ -1385,12 +1432,7 @@ impl App { source: input.to_string(), }]; } - self.push_output(OutputLine { - text: crate::t!("dsl.running", input = input), - kind: OutputKind::Echo, - mode_at_submission: mode, - styled_runs: None, - }); + self.push_output(OutputLine::echo(input, mode)); vec![Action::ExecuteDsl { command: cmd, source: input.to_string(), @@ -1401,11 +1443,15 @@ impl App { Err(err) => { // Echo the source line so the user can see what // got submitted (and copy-paste it back to fix). + // Parse error (not executed): plain `running:` echo, + // `status: None` — the caret aligns to `running: ` + // (ADR-0040 scope). self.push_output(OutputLine { text: crate::t!("dsl.running", input = input), kind: OutputKind::Echo, mode_at_submission: mode, styled_runs: None, + status: None, }); // Caret pointer at the failure position, when we // have one. Aligned to the "running: " prefix so @@ -1478,19 +1524,32 @@ impl App { /// Emit the standard `[ok] ` header used by /// every successful DSL command. Routes through the i18n /// catalog (ADR-0019 §9 sweep). + /// Resolve the oldest still-`Pending` echo to `status` + /// (ADR-0040). Results arrive in submission order (the db worker + /// is FIFO), so the oldest pending echo is this command's; a + /// finished command can never leave an earlier one stuck on + /// `running:`. No-op if there is no pending echo (e.g. a result + /// for a command whose echo path didn't mark one). + fn mark_oldest_pending_echo(&mut self, status: EchoStatus) { + if let Some(line) = self + .output + .iter_mut() + .find(|l| l.kind == OutputKind::Echo && l.status == Some(EchoStatus::Pending)) + { + line.status = Some(status); + } + } + + /// Mark a command's echo successful (ADR-0040 — replaces the old + /// `[ok] ` summary line) and emit the ADR-0038 + /// DSL → SQL teaching echo that was stashed on the success event. fn note_ok_summary(&mut self, command: &Command) { - self.note_system(crate::t!( - "ok.summary", - verb = command.verb(), - subject = command.display_subject() - )); - // ADR-0038 §4: the DSL → SQL teaching echo, beneath `[ok]`. - // Set on the success event when a DSL-form command ran in an - // advanced effective mode (ADR-0037); `None` otherwise. One - // `OutputKind::TeachingEcho` line per statement (§6 category - // 2): the dimmed `Executing SQL:` prefix + the SQL portion - // re-lexed in advanced mode for syntax highlighting — see - // `ui::render_output_line`'s `TeachingEcho` branch. + debug!(verb = command.verb(), "dsl command succeeded"); + self.mark_oldest_pending_echo(EchoStatus::Ok); + // ADR-0038 §4: one `OutputKind::TeachingEcho` line per + // statement — the dimmed `Executing SQL:` prefix + the SQL + // re-lexed in advanced mode for highlighting (see + // `ui::render_output_line`'s `TeachingEcho` branch). if let Some(lines) = self.pending_echo.take() { for line in lines { self.push_teaching_echo(&line); @@ -1688,18 +1747,13 @@ impl App { error = %rendered, "dsl command failed" ); - // Wrap the command portion in quotes so the message - // reads cleanly: "...failed: " rather than the - // command running into "failed: ..." with no break. - // `note_error` splits on newlines internally — refusal - // diagnostics from `change column …` (ADR-0017 §7) flow - // through as a multi-line bordered table. - self.note_error(crate::t!( - "dsl.failed", - verb = command.verb(), - subject = command.display_subject(), - rendered = rendered - )); + // ADR-0040: the echo line carries the ✗; the redundant + // `" " failed:` prefix is dropped — only the + // rendered reason is shown. `note_error` splits on newlines + // internally — refusal diagnostics from `change column …` + // (ADR-0017 §7) flow through as a multi-line bordered table. + self.mark_oldest_pending_echo(EchoStatus::Err); + self.note_error(crate::t!("dsl.failed", rendered = rendered)); } /// Construct a [`TranslateContext`] from a [`Command`] + schema- @@ -2305,6 +2359,7 @@ impl App { kind: OutputKind::TeachingEcho, mode_at_submission: self.mode, styled_runs: None, + status: None, }); } @@ -2348,6 +2403,7 @@ impl App { kind, mode_at_submission: self.mode, styled_runs: None, + status: None, }); return; } @@ -2357,6 +2413,7 @@ impl App { kind, mode_at_submission: self.mode, styled_runs: None, + status: None, }); } } @@ -3344,12 +3401,17 @@ mod tests { echo: Some(vec!["CREATE TABLE Other (id serial PRIMARY KEY)".to_string()]), }); let texts: Vec<&str> = app.output.iter().map(|l| l.text.as_str()).collect(); - let ok_idx = texts.iter().position(|t| t.starts_with("[ok]")).expect("an [ok] line"); + // ADR-0040: no `[ok]` summary; with no preceding `running:` echo + // (event fired directly), the teaching echo leads. + assert!( + !texts.iter().any(|t| t.starts_with("[ok]")), + "no [ok] summary line (ADR-0040): {texts:?}", + ); let echo_idx = texts .iter() .position(|t| t.contains("Executing SQL:")) .expect("an echo line"); - assert_eq!(echo_idx, ok_idx + 1, "echo sits immediately beneath [ok]: {texts:?}"); + assert_eq!(echo_idx, 0, "teaching echo leads the output: {texts:?}"); assert!(texts[echo_idx].contains("CREATE TABLE Other (id serial PRIMARY KEY)")); // No echo line when the event carries none (simple mode etc.). @@ -3382,15 +3444,19 @@ mod tests { fn assert_echo_beneath_ok(app: &App, expected: &str) { let texts: Vec<&str> = app.output.iter().map(|l| l.text.as_str()).collect(); - let ok_idx = texts - .iter() - .position(|t| t.starts_with("[ok]")) - .expect("an [ok] line"); + // ADR-0040: no `[ok]` summary line. These events are fired + // without a preceding `running:` echo (they bypass + // `dispatch_dsl`), so the teaching echo — pushed first by + // `note_ok_summary` — leads the output. + assert!( + !texts.iter().any(|t| t.starts_with("[ok]")), + "no [ok] summary line (ADR-0040): {texts:?}", + ); let echo_idx = texts .iter() .position(|t| t.contains("Executing SQL:")) .expect("an echo line"); - assert_eq!(echo_idx, ok_idx + 1, "echo sits immediately beneath [ok]: {texts:?}"); + assert_eq!(echo_idx, 0, "teaching echo leads the output: {texts:?}"); assert!(texts[echo_idx].contains(expected), "echo carries the SQL: {texts:?}"); // ADR-0038 §4 polish: every success arm now wires the echo as // `OutputKind::TeachingEcho` so `ui::render_output_line` fires @@ -3764,34 +3830,35 @@ mod tests { ]), }); let texts: Vec<&str> = app.output.iter().map(|l| l.text.as_str()).collect(); - let ok_idx = texts - .iter() - .position(|t| t.starts_with("[ok]")) - .expect("an [ok] line"); - // The three echo lines sit immediately beneath [ok], in order. + // ADR-0040: no `[ok]` summary; the event is fired without a + // preceding `running:` echo, so the three teaching echoes lead + // the output at indices 0/1/2, in order. assert!( - texts[ok_idx + 1].contains("Executing SQL: DROP INDEX Customers_Email_idx"), + !texts.iter().any(|t| t.starts_with("[ok]")), + "no [ok] summary line (ADR-0040): {texts:?}", + ); + assert!( + texts[0].contains("Executing SQL: DROP INDEX Customers_Email_idx"), "first echo line: {texts:?}", ); assert!( - texts[ok_idx + 2].contains("Executing SQL: DROP INDEX Customers_Email_Day_idx"), + texts[1].contains("Executing SQL: DROP INDEX Customers_Email_Day_idx"), "second echo line: {texts:?}", ); // ADR-0038 §4 polish: every one of the multi-statement echo lines // carries TeachingEcho — the polish styling fires per line. A // regression that pushed the multi-line case as System would // leave the text intact but break the per-line styling. - for (offset, label) in [(1, "first"), (2, "second"), (3, "third")] { + for (idx, label) in [(0, "first"), (1, "second"), (2, "third")] { assert_eq!( - app.output[ok_idx + offset].kind, + app.output[idx].kind, OutputKind::TeachingEcho, "{label} echo line carries TeachingEcho: {:?}", - app.output[ok_idx + offset], + app.output[idx], ); } assert!( - texts[ok_idx + 3] - .contains("Executing SQL: ALTER TABLE Customers DROP COLUMN Email"), + texts[2].contains("Executing SQL: ALTER TABLE Customers DROP COLUMN Email"), "third echo line: {texts:?}", ); // Pin the `Executing SQL:` prefix repeats once per statement @@ -3981,8 +4048,13 @@ mod tests { app.output.iter().any(|l| l.text.contains("id")), "expected `id` somewhere in structure output", ); - // Earlier line is the [ok] header. - assert!(app.output.iter().any(|l| l.text.starts_with("[ok]"))); + // ADR-0040: no `[ok]` summary line — success is signalled by + // the echo's ✓ marker (no echo pushed in this direct-event + // test) and the structure render itself. + assert!( + !app.output.iter().any(|l| l.text.starts_with("[ok]")), + "no [ok] summary line (ADR-0040)", + ); } #[test] @@ -4007,11 +4079,12 @@ mod tests { command: cmd, plan, }); - // `[ok] explain Customers` header. + // ADR-0040: no `[ok] explain …` header — the (no-echo here) + // command's success shows via the marker; the plan output + // itself carries the content. assert!( - app.output.iter().any(|l| l.text.starts_with("[ok]") - && l.text.contains("explain")), - "expected an [ok] explain header", + !app.output.iter().any(|l| l.text.starts_with("[ok]")), + "no [ok] summary line (ADR-0040)", ); // The display SQL and the plan node both reach output. assert!( @@ -4026,6 +4099,79 @@ mod tests { ); } + // ---- ADR-0040: completion marker on the echo line ---------- + + #[test] + fn mark_oldest_pending_echo_resolves_in_submission_order() { + // Two echoes in flight; results arrive in submission order, so + // the oldest pending echo is the correct target each time — a + // finished command never leaves an earlier one stuck Pending. + let mut app = App::new(); + app.output + .push_back(OutputLine::echo("first", crate::mode::Mode::Advanced)); + app.output + .push_back(OutputLine::echo("second", crate::mode::Mode::Advanced)); + app.mark_oldest_pending_echo(EchoStatus::Ok); + app.mark_oldest_pending_echo(EchoStatus::Err); + let echoes: Vec<_> = app + .output + .iter() + .filter(|l| l.kind == OutputKind::Echo) + .collect(); + assert_eq!(echoes[0].status, Some(EchoStatus::Ok), "first → Ok"); + assert_eq!(echoes[1].status, Some(EchoStatus::Err), "second → Err"); + } + + #[test] + fn successful_command_resolves_its_echo_to_ok_with_no_summary_line() { + // Full flow: dispatch pushes a Pending echo; the success event + // resolves it to ✓ and emits no `[ok]` summary line (ADR-0040). + let mut app = App::new(); + type_str(&mut app, "create table T with pk"); + submit(&mut app); + let echo = app + .output + .iter() + .find(|l| l.kind == OutputKind::Echo) + .expect("dispatch pushed an echo"); + assert_eq!(echo.status, Some(EchoStatus::Pending), "pending before result"); + app.update(AppEvent::DslSucceeded { + command: Command::CreateTable { + name: "T".to_string(), + columns: vec![crate::dsl::ColumnSpec::new("id", Type::Serial)], + primary_key: vec!["id".to_string()], + }, + description: None, + echo: None, + }); + let echo = app + .output + .iter() + .find(|l| l.kind == OutputKind::Echo) + .expect("echo still present"); + assert_eq!(echo.status, Some(EchoStatus::Ok), "resolved to Ok"); + assert!( + !app.output.iter().any(|l| l.text.starts_with("[ok]")), + "no [ok] summary line", + ); + } + + #[test] + fn parse_error_echo_stays_pending_and_keeps_running_prefix() { + // ADR-0040 scope: a parse error never reaches the worker, so + // its echo is not marker-tracked (status None) and keeps the + // `running:` rendering + caret. + let mut app = App::new(); + type_str(&mut app, "frobnicate widgets"); + submit(&mut app); + let echo = app + .output + .iter() + .find(|l| l.kind == OutputKind::Echo) + .expect("parse error still echoes the input"); + assert_eq!(echo.status, None, "parse-error echo is not marker-tracked"); + } + #[test] fn replay_command_dispatches_replay_action_not_execute_dsl() { // Submitting `replay ` must NOT produce an @@ -4951,12 +5097,17 @@ mod tests { // ADR-0033 sub-phase 3f: a SQL DELETE reuses the DSL delete // renderer (CommandOutcome::Delete -> handle_dsl_delete_ // success). This pins that the SHARED renderer produces the - // right user-facing strings for the SQL path — the ok-summary - // (verb + subject, where SqlDelete's subject is its target - // table) and the per-relationship cascade line. The integration - // tests check the DeleteResult struct; this checks the render. + // right user-facing strings for the SQL path — the row count + // and the per-relationship cascade line. ADR-0040: success is + // signalled by the echo's ✓ marker (no more `[ok]` summary), so + // we push the `running:` echo first (as `dispatch_dsl` does) + // and assert it resolves to `Ok`. use crate::dsl::ReferentialAction; let mut app = App::new(); + app.output.push_back(OutputLine::echo( + "delete from Customers where id = 1", + crate::mode::Mode::Advanced, + )); app.update(AppEvent::DslDeleteSucceeded { command: Command::SqlDelete { sql: "delete from Customers where id = 1".to_string(), @@ -4981,9 +5132,16 @@ mod tests { echo: None, }); let texts: Vec = app.output.iter().map(|l| l.text.clone()).collect(); + // ADR-0040: the echo resolves to ✓ (Ok); no `[ok]` summary line. assert!( - texts.iter().any(|t| t.contains("delete from") && t.contains("Customers")), - "ok summary names the verb + target table: {texts:?}", + app.output + .iter() + .any(|l| l.kind == OutputKind::Echo && l.status == Some(EchoStatus::Ok)), + "the echo line resolves to Ok: {texts:?}", + ); + assert!( + !texts.iter().any(|t| t.starts_with("[ok]")), + "no [ok] summary line (ADR-0040): {texts:?}", ); assert!( texts.iter().any(|t| t.contains("1 row(s) deleted")), diff --git a/src/friendly/keys.rs b/src/friendly/keys.rs index 313a657..1cc63d4 100644 --- a/src/friendly/keys.rs +++ b/src/friendly/keys.rs @@ -393,7 +393,7 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("archive.unsafe_entry", &["entry"]), ("archive.zip", &["path", "message"]), // ---- DSL failure wrapper / running echo ---- - ("dsl.failed", &["verb", "subject", "rendered"]), + ("dsl.failed", &["rendered"]), ("dsl.running", &["input"]), // ---- Persistence-fatal banner ---- ("fatal.persistence", &["operation", "path", "message"]), @@ -529,7 +529,6 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("ok.rows_deleted", &["count"]), ("ok.rows_inserted", &["count"]), ("ok.rows_updated", &["count"]), - ("ok.summary", &["verb", "subject"]), // ---- Client-side success notes (ADR-0017 §6, ADR-0018 §9) ---- ("client_side.auto_fill_add_serial", &["count"]), ("client_side.auto_fill_add_shortid", &["count"]), diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index 3e4b37d..315feff 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -665,12 +665,15 @@ project: # ---- DSL failure wrapper + advanced-mode placeholder + fatal -------- dsl: - # Wrapper around the friendly-error layer's translated - # message, surfaced as `" " failed: `. - failed: '"{verb} {subject}" failed: {rendered}' + # The friendly-error layer's translated reason, shown beneath the + # failed command's echo line. ADR-0040: the redundant + # `" " failed:` prefix was dropped — the echo line + # carries the ✗ marker and names the command. + failed: "{rendered}" # Echo line `running: ` shown above each command's - # response so the user has on-screen context for the - # output that follows. + # response while it executes; ADR-0040 resolves it to + # ` ✓` / ` ✗` on completion (the marker replaces + # the old `[ok] ` summary line). running: "running: {input}" # ---- Value-validation errors (per-column at bind time) -------------- @@ -916,12 +919,11 @@ db: # locale lands.) ok: - # Generic `[ok] ` header used for every - # successful DSL command. Verbs come from `Command::verb()` - # (already English keywords); subjects are the table / - # relationship endpoints derived in `Command::display_subject()`. - summary: "[ok] {verb} {subject}" - # Per-operation row-count footers shown beneath the summary. + # ADR-0040: the generic `[ok] ` summary line was + # retired — a successful command's echo line now carries a ✓ + # marker (the verb+subject duplicated the echo above it). The + # per-operation row-count footers below still convey real payload + # and are unchanged. rows_inserted: " {count} row(s) inserted" rows_updated: " {count} row(s) updated" rows_deleted: " {count} row(s) deleted" diff --git a/src/snapshots/rdbms_playground__ui__tests__populated_with_table_dark.snap b/src/snapshots/rdbms_playground__ui__tests__populated_with_table_dark.snap index bbb52b7..4536ba5 100644 --- a/src/snapshots/rdbms_playground__ui__tests__populated_with_table_dark.snap +++ b/src/snapshots/rdbms_playground__ui__tests__populated_with_table_dark.snap @@ -1,10 +1,10 @@ --- source: src/ui.rs -assertion_line: 1771 +assertion_line: 1841 expression: snapshot --- ╭ Tables ──────────────────╮╭ Output ──────────────────────────────────────────╮ -│Customers ││[system] [ok] create table Customers │ +│Customers ││[simple] create table Customers ✓ │ │Orders ││[system] Customers │ │ ││[system] id serial [PK] │ │ ││[system] Name text │ diff --git a/src/ui.rs b/src/ui.rs index 8959ea5..1429ae3 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -12,7 +12,7 @@ use ratatui::style::{Modifier, Style}; use ratatui::text::{Line, Span}; use ratatui::widgets::{Block, BorderType, Borders, Paragraph, Wrap}; -use crate::app::{App, EffectiveMode, OutputKind, OutputLine, OutputStyleClass}; +use crate::app::{App, EchoStatus, EffectiveMode, OutputKind, OutputLine, OutputStyleClass}; use crate::mode::Mode; use crate::theme::Theme; @@ -718,7 +718,24 @@ fn approximate_wrapped_rows_from_output( OutputKind::System | OutputKind::TeachingEcho => "[system] ".len(), OutputKind::Error => "[error] ".len(), }; - let total = tag_len + line.text.chars().count(); + // ADR-0040: a completed echo renders ` ✓/✗` — + // the `running: ` prefix is dropped and a 2-column marker + // (space + glyph) appended; everything else renders its + // text verbatim. + let content_chars = if matches!( + (line.kind, line.status), + (OutputKind::Echo, Some(EchoStatus::Ok | EchoStatus::Err)) + ) { + line.text + .strip_prefix(crate::dsl::ECHO_PREFIX) + .unwrap_or(line.text.as_str()) + .chars() + .count() + + 2 + } else { + line.text.chars().count() + }; + let total = tag_len + content_chars; if total == 0 { 1 } else { total.div_ceil(w) } }) .sum() @@ -753,31 +770,51 @@ fn render_output_line<'a>(line: &'a OutputLine, theme: &Theme) -> Line<'a> { OutputKind::Error => "[error] ".to_string(), }; - // Simple-mode echo lines get token-class highlighting on - // their input portion (ADR-0022 §5). Echo body shape is - // contracted to ``; the prefix is - // pinned to the catalog template by + // ADR-0040: an echo renders `running: ` while pending + // (and for untracked parse/pre-flight rejections), and + // ` ✓` / ` ✗` once an executed command completes + // — the marker replaces the old `[ok]`/`failed:` summary line. + // Simple-mode input keeps its token-class highlighting (ADR-0022 + // §5); advanced-mode input renders plain, as before. The body + // shape is ``; the prefix is pinned to the + // catalog template by // `dsl::tests::echo_prefix_matches_catalog_template`. - if line.kind == OutputKind::Echo - && line.mode_at_submission == Mode::Simple - && let Some(rest) = line.text.strip_prefix(crate::dsl::ECHO_PREFIX) - { - let mut spans: Vec> = Vec::with_capacity(2 + rest.len() / 4); + if line.kind == OutputKind::Echo { + let input = line + .text + .strip_prefix(crate::dsl::ECHO_PREFIX) + .unwrap_or(line.text.as_str()); + let mut spans: Vec> = Vec::with_capacity(3 + input.len() / 4); spans.push(Span::styled(tag, tag_style)); - spans.push(Span::styled( - crate::dsl::ECHO_PREFIX, - Style::default().fg(theme.fg), - )); - for run in crate::input_render::lex_to_runs(rest, theme) { + // Pending / untracked → keep the `running: ` prefix; + // completed → drop it (the marker carries the outcome). + if !matches!(line.status, Some(EchoStatus::Ok | EchoStatus::Err)) { spans.push(Span::styled( - &rest[run.byte_range.0..run.byte_range.1], - run.style, + crate::dsl::ECHO_PREFIX, + Style::default().fg(theme.fg), )); } + if line.mode_at_submission == Mode::Simple { + for run in crate::input_render::lex_to_runs(input, theme) { + spans.push(Span::styled( + &input[run.byte_range.0..run.byte_range.1], + run.style, + )); + } + } else { + spans.push(Span::styled(input, Style::default().fg(theme.fg))); + } + match line.status { + Some(EchoStatus::Ok) => { + spans.push(Span::styled(" ✓", Style::default().fg(theme.system))); + } + Some(EchoStatus::Err) => { + spans.push(Span::styled(" ✗", Style::default().fg(theme.error))); + } + _ => {} + } return Line::from(spans); } - // Echo body without the expected prefix, or any non-echo - // line, falls through to the plain rendering below. // ADR-0038 §4 styled-runs polish — the DSL → SQL teaching echo // gets the same syntactic treatment as the input echo, but with @@ -1293,6 +1330,7 @@ mod tests { kind: OutputKind::TeachingEcho, mode_at_submission: Mode::Advanced, styled_runs: None, + status: None, }; let rendered = render_output_line(&line, &theme); // [system] tag, then the dim prefix, then ≥1 SQL spans. @@ -1397,6 +1435,7 @@ mod tests { kind: OutputKind::System, mode_at_submission: Mode::Simple, styled_runs: None, + status: None, }; let rendered = render_output_line(&line, &theme); // tag span + single whole-line body span. @@ -1614,6 +1653,34 @@ mod tests { insta::assert_snapshot!("rebuild_confirm_modal_dark", snapshot); } + // ---- ADR-0040: echo completion marker ----------------------- + + #[test] + fn echo_renders_running_then_marker_per_status() { + use crate::app::EchoStatus; + let mut app = App::new(); + // Pending → `running: ` (current look). + app.output + .push_back(OutputLine::echo("drop table Orders", Mode::Advanced)); + // Ok → ` ✓`, no `running:`. + let mut ok = OutputLine::echo("create table T with pk", Mode::Simple); + ok.status = Some(EchoStatus::Ok); + app.output.push_back(ok); + // Err → ` ✗`, no `running:`. + let mut err = OutputLine::echo("insert into T values (1)", Mode::Advanced); + err.status = Some(EchoStatus::Err); + app.output.push_back(err); + + let out = render_to_string(&mut app, &Theme::dark(), 100, 20); + assert!(out.contains("running: drop table Orders"), "pending keeps running::\n{out}"); + assert!(out.contains("create table T with pk ✓"), "ok shows ✓:\n{out}"); + assert!(out.contains("insert into T values (1) ✗"), "err shows ✗:\n{out}"); + assert!( + !out.contains("running: create table"), + "a completed echo drops the running: prefix:\n{out}" + ); + } + // ---- Issue #13: undo confirm dialog ------------------------- #[test] @@ -1741,30 +1808,32 @@ mod tests { check_constraints: Vec::new(), }; app.current_table = Some(desc); - // Mirror what the App writes when a DSL command succeeds. - app.output.push_back(OutputLine { - text: "[ok] create table Customers".to_string(), - kind: OutputKind::System, - mode_at_submission: Mode::Simple, - styled_runs: None, - }); + // Mirror what the App writes when a DSL command succeeds + // (ADR-0040): the command's echo line resolves to a ✓ marker — + // there is no separate `[ok]` summary line. + let mut echo = OutputLine::echo("create table Customers", Mode::Simple); + echo.status = Some(crate::app::EchoStatus::Ok); + app.output.push_back(echo); app.output.push_back(OutputLine { text: " Customers".to_string(), kind: OutputKind::System, mode_at_submission: Mode::Simple, styled_runs: None, + status: None, }); app.output.push_back(OutputLine { text: " id serial [PK]".to_string(), kind: OutputKind::System, mode_at_submission: Mode::Simple, styled_runs: None, + status: None, }); app.output.push_back(OutputLine { text: " Name text".to_string(), kind: OutputKind::System, mode_at_submission: Mode::Simple, styled_runs: None, + status: None, }); let theme = Theme::dark(); diff --git a/tests/walking_skeleton.rs b/tests/walking_skeleton.rs index d093585..db442d2 100644 --- a/tests/walking_skeleton.rs +++ b/tests/walking_skeleton.rs @@ -306,9 +306,11 @@ fn create_table_flow_updates_tables_list_and_structure_view() { rendered.contains("Customers"), "items panel should list Customers:\n{rendered}" ); + // ADR-0040: success is the ✓ marker on the command's echo line + // (the `[ok] create table Customers` summary line was retired). assert!( - rendered.contains("[ok] create table Customers"), - "output should confirm success:\n{rendered}" + rendered.contains("create table Customers with pk ✓"), + "the command echo should resolve to a success marker:\n{rendered}" ); // The structure table renders one line per column; the // `id` row shows both the name and its `serial` type @@ -397,7 +399,12 @@ fn drop_table_flow_clears_items_list() { assert!(app.current_table.is_none()); let rendered = rendered_text(&mut app, &Theme::dark(), 80, 24); assert!(rendered.contains("(none yet)")); - assert!(rendered.contains("[ok] drop table Customers")); + // ADR-0040: `drop table` is content-less, so the echo's ✓ marker + // is the entire success signal (replacing `[ok] drop table …`). + assert!( + rendered.contains("drop table Customers ✓"), + "the drop echo should resolve to a success marker:\n{rendered}" + ); } #[test]