diff --git a/docs/adr/0037-execution-time-mode-side-channel.md b/docs/adr/0037-execution-time-mode-side-channel.md index 4b32a8e..318283e 100644 --- a/docs/adr/0037-execution-time-mode-side-channel.md +++ b/docs/adr/0037-execution-time-mode-side-channel.md @@ -203,8 +203,74 @@ Two refinements found when building, recorded so the ADR matches reality: `EffectiveMode`) is Tier-1 testable, and the worker-side threading becomes live + end-to-end testable the moment the echo reads it. +## Amendment 1 — Output tag is colour-coded by status, not mode (2026-05-31, issue #10) + +The original side-channel (§ above) exists "purely to label the +**input-echo line**" — the `[simple]`/`[advanced]` tag whose colour +tells the learner which mode their command ran under. In +implementation, however, the tag-colour rule in `render_output_line` +(`src/ui.rs`) was applied to **every** output kind, keyed on +`mode_at_submission` regardless of whether the line was an echo. That +over-applied the channel: a `[system]` line and an `[error]` line — +neither of which is an input echo — both picked up the same mode tint +(blue in simple, orange in advanced). The only thing distinguishing a +routine `[system]` message from an `[error]` was the **body** colour +(green vs red), while the tag — the leftmost glyph the eye lands on — +was identical (issue #10). + +That is backwards for the line a learner most needs to spot fastest. +The mode has limited value on an error line; "this is an error" has +high value. And flooding the whole error **body** in red makes a long +message *harder* to read, not easier. + +**Change — the status-coloured-tag model.** The output tag is +colour-coded by the message's **status** (its `OutputKind`), and the +**body** is neutral so the message text stays readable: + +| Kind | Tag colour | Body | +| --- | --- | --- | +| `Echo` | **mode tint** (`mode_simple`/`mode_advanced`) — *the sole exception* | `theme.fg` / lexed (unchanged); per-command success rides the trailing ✓/✗ (ADR-0040) | +| `System` | `theme.system` (green) | `theme.fg` (was green) | +| `TeachingEcho` | `theme.system` (green — it is a `[system]`-tagged line) | dim prefix + lexed SQL (unchanged) | +| `Error` | `theme.error` (red) | `theme.fg` **+ BOLD** (was red) | + +This **narrows** the side-channel to its stated purpose rather than +contradicting it: the mode tint now lives **only** on the echo tag, +where ADR-0037 always said it belonged. Everything else reads as a +status traffic-light — **green tag = ok/info, red tag = error** — +which is the same palette as the ✓/✗ echo markers (ADR-0040), so the +whole output surface speaks one colour vocabulary. + +**Why bold-neutral for the error body** (not plain, not red). This is +the established diagnostic-rendering convention — `rustc`, `clang`, +`tsc`, and most linters colour the **severity label** and render the +**message** in the default foreground (bold), not a wall of severity +colour. The red moves to the tag (the scan target); the body keeps +weight via BOLD without the readability cost of coloured prose. + +**Scope / non-changes.** + +- `OutputLine.mode_at_submission` is **unchanged** — still carries the + mode for the echo tag. Only *which kinds consult it for colour* changed. +- The ✓/✗ completion markers (ADR-0040) are untouched — they already + use `theme.system`/`theme.error` directly, and now visually rhyme + with the new tag colours. +- This supersedes the three options sketched in issue #10 (red tag / + amber attention tag / glyph) with a cleaner fourth model that also + fixes body readability and the `[system]` tag in one rule. ADR-0040 + had flagged the `[error]`/`[system]` tag colours as orthogonal and + out of its scope (issue #10) — this amendment closes that gap. + +**Coverage** (`src/ui.rs` tests): `system_line_renders_green_tag_and_neutral_body`, +`error_line_renders_red_tag_and_bold_neutral_body`, +`echo_tag_keeps_the_mode_tint_not_a_status_colour` (locks the sole +exception across both modes), `teaching_echo_tag_is_green_like_other_system_lines`. + ## See also +- ADR-0040 — the ✓/✗ completion markers whose green/red palette the + status tag now matches; it deferred these tag colours as orthogonal + (issue #10), closed by Amendment 1. - ADR-0033 Amendment 3 — deferred this side-channel; defines the intrinsic command-identity model this ADR must not disturb. - ADR-0030 §10 — the DSL → SQL teaching bridge (the motivating consumer). diff --git a/docs/adr/README.md b/docs/adr/README.md index b2c66d1..638cc00 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -42,7 +42,7 @@ This directory contains the project's ADRs, recorded per - [ADR-0034 — `history.log` as a complete command journal; replay reads success-only](0034-history-journal-and-replay-filter.md) — **Accepted**, resolves a three-way tension in `history.log`'s roles found while implementing ADR-0033 3f: (1) the persistent log is success-only while the in-memory Up/Down recall ring records *every* submission (success or failure, "so users can recall and edit typo'd commands"), and the ring is re-seeded from the log on project open — so **failed commands are recallable within a session but silently lost across sessions**; (2) replay wants the state-building (successful) commands while recall wants everything typed, which one success-only file cannot serve; (3) `replay history.log` never actually worked — `run_replay` parses each whole line through the DSL parser with no understanding of the `||` record shape, so a real log fails on line 1, and **no test ever fed the pipe format to replay** (the `replay_history_log_records_subcommands_only` test only checks what replay *writes*, never replays the log as input). Decision: `history.log` becomes a **complete journal** — every submission recorded, tagged `ok`/`err` via the status field the format already reserved (ADR-0015 §5) — and **each consumer filters**: hydration reads all records (cross-session recall matches in-session), replay reads `ok` only (and learns the journal format, while still accepting bare-command `.commands` scripts; detection by the leading timestamp+status prefix so a `|` inside a bare command isn't misread). Successful commands stay journalled transactionally by the worker; failed commands are journalled `err` best-effort from the runtime/app error path (a parse failure never reaches the worker). Amends ADR-0006's "successfully executed" wording and ADR-0015 §5 ("status always `ok`") / §12 (hydration). Code deferred to two tracked test-first sub-tasks (journal-failures+filtering; replay-parses-journal-format); existing all-`ok` logs need no migration; **implemented 2026-05-24** (plan `docs/plans/20260524-adr-0034-history-journal.md`); **Amendment 1 (2026-05-24): replay filters out app-lifecycle commands** — a working `replay history.log` (the §3 fix) exposed that the journal also records `save as`/`load`/`new`/`export`/`import`/`rebuild`/`mode` (which would panic the worker dispatch or abort the replay), so replay now re-applies **only** schema/data write commands and **skips** every `Command::App` + nested `Command::Replay`; **all skips continue** (never abort — reversing the prior nested-`replay` refusal, so a journal containing a once-run `replay` needs no hand-editing, and the infinite-loop footgun is closed by construction), with a `[skip]` **warning** on `import` and nested-`replay` skips (their omission can leave replayed state incomplete) and silent skips for the rest; `replay.error_nested` removed, `replay.skipped_import`/`replay.skipped_replay` added, `ReplayCompleted` carries `warnings` - [ADR-0035 — Advanced-mode SQL DDL](0035-advanced-mode-sql-ddl.md) — **Accepted** (design agreed 2026-05-24; validated end-to-end by sub-phases 4a/4a.2/4a.3/4b `CREATE TABLE` (incl. foreign keys) + 4c `DROP TABLE [IF EXISTS]` + 4d `CREATE [UNIQUE] INDEX` / `DROP INDEX [IF EXISTS]` + 4e `ALTER TABLE` add/drop/rename column + 4f `ALTER TABLE … ALTER COLUMN TYPE` + 4g `ALTER TABLE` add/drop constraint + add FK + 4h `ALTER TABLE … RENAME TO` + 4i verification sweep (completion merge + simple/advanced completion colour + describe of table-level constraints + self-ref FK indicator + CREATE-TABLE help/usage), implemented 2026-05-25/26 — **Phase 4 complete**; **Amendment 1, 2026-05-26**: drop a composite UNIQUE via a derived, engine-neutral `unique_` name that reuses the existing `DROP CONSTRAINT ` grammar — no new syntax, no metadata, §4g anonymity intact; `describe` shows the name; dropping a UNIQUE-covered *column* now refuses with that name + the drop command), **Phase 4** of the ADR-0030 roadmap (peer of 0031/0032/0033) and **clarifies ADR-0030 §4**. Advanced-mode `CREATE`/`DROP`/`ALTER TABLE` + `CREATE`/`DROP INDEX` get their **own per-statement commands** (`SqlCreateTable`/`SqlAlterTable`/`SqlDropTable`/`SqlCreateIndex`/`SqlDropIndex`), like DML's `Sql*` set — but unlike DML they **execute *structurally*, not verbatim** (raw execution would lose the playground's types, named relationships, and `STRICT`; "verbatim" was a DML convenience, not a rule). Handlers **reuse the low-level schema/metadata helpers** where the operation matches simple mode and **stand alone where the SQL surface is richer** (clarity over forced refactoring); simple mode is untouched (additive). Dispatch: `create`/`drop` reuse ADR-0033 Amendment 1's category-grouped mode-aware dispatch (SQL-first, simple fallback); `alter` is a new advanced-only entry word. Full surface (no pre-emptive cuts, `Q4`): `CREATE TABLE` with column + table constraints, single/compound `PRIMARY KEY`, inline + table-level `FOREIGN KEY` → **named relationships** (one statement = one command = **one undo step**, ADR-0006); `ALTER TABLE` add/drop/rename column, `ALTER COLUMN TYPE`, add/drop constraint, add FK, **`RENAME TO`** (advanced-only table rename — new low-level op renaming the table + its CSV + the relationship and table-CHECK metadata, closing the rename half of `C1`); `CREATE [UNIQUE] INDEX` / `DROP INDEX`. Type slot accepts the ten playground keywords **and** standard-SQL aliases (`integer`→`int`, `varchar`→`text`, `timestamp`→`datetime`, …; length args accepted-and-ignored; no engine type names in/out — ADR-0030 §5). `CHECK`/`DEFAULT` reuse ADR-0031 `sql_expr`. **Pre-implementation `/runda` refinements (2026-05-24, user-confirmed):** `CREATE TABLE`/`DROP TABLE` **admit `IF [NOT] EXISTS`** (no-op-that-succeeds-with-a-note — a near-universal cross-vendor idiom, reclassified *into* scope, not engine-specific); `INTEGER PRIMARY KEY` maps to a **plain `int`** PK, *not* auto-increment (`serial` stays the sole auto-increment type). **Column-type-conversion is unified** (ADR-0017 engine, mode-appropriate policy): clean auto-converts and incompatible/own-type-static cases refuse in both modes, but a **lossy** change refuses-by-default in simple mode (`--force-conversion` opts in) while advanced mode **performs it with a loss note** and relies on **`undo` as the safety net** — no force flag, no dropping to simple mode (a payoff of shipping ADR-0006 first). OOS: views/triggers/txn-control/PRAGMA/etc. (ADR-0030 §3), the Postgres `USING` clause, and the DSL→SQL teaching echo (ADR-0030 Phase 5). Sub-phases 4a–4i, plus **4a.2** (per-column `CHECK`/`DEFAULT` via raw `sql_expr` text — `sql_expr` is validate-only, no `Expr` AST — + composite `UNIQUE(a,b)`; no new internal table) and **4a.3** (table-level/multi-column `CHECK`, landed via the new `__rdbms_playground_table_checks` metadata table because SQLite has no PRAGMA for CHECK; the builder tells a table-level CHECK from a column-level one by element position) and **4b** (foreign keys — inline `REFERENCES` + table-level `FOREIGN KEY` → ADR-0013 named relationships in the create transaction, one undo step; self-references + bare `REFERENCES ` supported, user-confirmed) and **4c** (`DROP TABLE [IF EXISTS]` → `SqlDropTable`, reusing `do_drop_table`; `IF EXISTS` is a no-op-with-note via `DropOutcome::Skipped`) and **4d** (`CREATE [UNIQUE] INDEX [IF NOT EXISTS] [] ON (cols)` → `SqlCreateIndex` and `DROP INDEX [IF EXISTS] ` → `SqlDropIndex`, reusing `do_add_index`/`do_drop_index`; **`CREATE UNIQUE INDEX` admitted** — ADR-0025 **Amendment 1** — via an additive `IndexSchema.unique` flag that round-trips through `project.yaml` and rebuild, with `[unique]` markers in the structure view + items panel, while simple-mode `add unique index` stays deferred; `IF [NOT] EXISTS` reuses the 4c skip path; `create`/`drop` each gain a *second* advanced node, exercising the all-candidates dispatch) and **4e** (`ALTER TABLE` add/drop/rename column → `SqlAlterTable`; `alter` is a new advanced-**only** entry word, runtime-decomposed to the existing `do_add_column`/`do_drop_column`/`do_rename_column` — no new worker layer; `do_add_column` extended to consume raw `default_sql`/`check_sql` so ADD COLUMN reaches CREATE-TABLE constraint parity; drop/rename refuse a column any CHECK references (table-level AND column-level, incl. a column's own self-check on rename) — the 4a.3 deferral, via a raw-CHECK-text tokenizer in the shared executors, so it guards both surfaces and fixes a latent rename-drift bug; SQL DROP COLUMN refuses an index-covered column with no `--cascade` spelling; the column executors + `do_add_index` gained an internal-`__rdbms_*`-table guard — all user-confirmed) and **4f** (`ALTER TABLE … ALTER COLUMN TYPE` → a fourth `AlterTableAction`, runtime-decomposed to the existing `change_column_type` with `ChangeColumnMode::ForceConversion` — which **is** the §7 advanced policy: lossy converts *with a note* (no force flag), incompatible + ADR-0017 static refusals (`↔ blob`, same-type, `date ↔ datetime`, non-`int → serial`) still refuse, while **`int → serial` is allowed** (auto-fills nulls + UNIQUE, ADR-0018 §8 — the §7 "→serial refused" summary is looser than the code); the builder discriminates the fourth branch by the **`type` keyword** (unique — ADD COLUMN's type is an ident), the type slot reuses `SQL_TYPE`; the internal-`__rdbms_*` guard was folded into `do_change_column_type`, closing the simple `change column` exposure too — user-confirmed) and **4g** (`ALTER TABLE … ADD [CONSTRAINT ] (CHECK | UNIQUE | FOREIGN KEY)` + `DROP CONSTRAINT `; ADD = CHECK + composite UNIQUE + FK, with `ADD PRIMARY KEY` and a *named* UNIQUE refused — composite UNIQUE is anonymous in our model; each ADD reuses a low-level path (table-CHECK/UNIQUE rebuild with a dry-run guard; FK → `add_relationship`, bare `REFERENCES

` → parent single-PK), DROP CONSTRAINT resolves the name to a table-CHECK then a child-side FK; **named table-CHECKs round-trip** via a nullable `name` column on `__rdbms_playground_table_checks` (**rebuild-only** arrival — pre-4g projects gain it on `rebuild`, a named add on an un-upgraded project is refused with a friendly "rebuild first" message) *and* a `project.yaml` `check_constraints` extension to an `{expr, name}` mapping (the bare-string form still reads); the internal-`__rdbms_*` guard was folded into `do_add_constraint`/`do_add_relationship`, completing that guard class — all user-confirmed) and **4h** (`ALTER TABLE … RENAME TO` — the one genuinely new low-level op, `do_rename_table`: a native engine rename plus one-transaction reconciliation of every metadata row naming the table (`__rdbms_playground_columns`, **both ends** of `__rdbms_playground_relationships`, `__rdbms_playground_table_checks`), the CSV file (the existing rewrite+delete path — no new persistence method), and **CHECK text that qualifies a column with the old table name** (`T.age`→`U.age`, a planning-`/runda` finding — the engine rewrites the live CHECK but the stored text would drift and break a fresh rebuild; `rewrite_check_table_qualifier` keeps them in step); grammar splits the `rename` verb into one branch with an inner Choice on a distinct second keyword (`column` vs `to`), the new-name slot mirroring the `CREATE TABLE` name slot; refuses same-name / existing-target / `__rdbms_*` / non-existent, with **case-insensitive** collision checks behind an engine-neutral pre-check (a finished-slice `/runda` finding — the engine matches names case-insensitively); auto-named indexes *and* relationships keep their stale names (only table-name columns update — §6 scope); one undo step; advanced-only, closing the rename half of `C1` — all user-confirmed) and **4i** (the verification sweep that completes Phase 4: the shared-entry-word completion merge + the simple-vs-advanced completion colour-when-mixed with Both→Advanced→Simple block ordering; `describe` of table-level composite UNIQUE + table CHECK; the self-ref FK pre-submit indicator fix; and the CREATE-TABLE help/usage skeleton refresh). **All of Phase 4 (4a–4i) is shipped.** Each sub-phase has exit + DA gates; **Amendment 2, 2026-05-27** (design agreed, pending impl): a **standard-first dialect stance** (refines ADR-0030's "standard SQL" posture — ISO spelling is canonical + echoed where one exists; a vendor shorthand may be *accepted* but isn't canonical; where ISO offers none, *one* documented vendor spelling is a deliberate extension) + an `ALTER COLUMN` **constraint gap-fill** surfaced by the ADR-0038 echo design: makes ISO `ALTER COLUMN … SET DATA TYPE` the canonical type-change verb with `TYPE` retained as a synonym (**reverses §4f's "no `SET DATA TYPE`"**), and adds `SET/DROP DEFAULT` (ISO) + `SET/DROP NOT NULL` (the one documented extension — ISO has no in-place NOT-NULL verb; PostgreSQL's chosen for being type-independent), all **rebuild-backed via the existing ADR-0029 `do_add_constraint`/`do_drop_constraint` executors** (dry-run + internal-table guards free, no new worker layer), reaching simple↔advanced constraint-mod **parity for NOT NULL + DEFAULT**; the **rebuild stays hidden** (Category-1 engine detail, ADR-0038). Residual gap left open + flagged: dropping a **column-level (anonymous) UNIQUE/CHECK** (no portable name — same class as Am1's parallel gap), which ADR-0038's catalogue marks "no headline echo" - [ADR-0036 — Value validation for advanced-mode DML](0036-typed-dml-values-vs-verbatim.md) — **Accepted** (design agreed + `/runda`'d 2026-05-26; mechanism then **deliberately narrowed** from "bind literals via the DSL path" to surgical **"validate-and-retain, execute verbatim"** after the user resisted consolidating the modes and a concrete auto-fill difference confirmed even the single-row literal case isn't identical across modes; **Phases 1–2 implemented** 2026-05-26 — `INSERT … VALUES` and `UPDATE … SET` literal validation + offending-value retention, capture-at-parse with no grammar change; **Phase 3a implemented** 2026-05-26 — live typed-slot hints + numeric-shape highlighting for `UPDATE`/UPSERT `SET col = ` via a boundary-aware lookahead (Amendment 1 corrects this ADR's naive-`Choice` sketch); **Phase 3b implemented** 2026-05-27 — per-position typed slots for `INSERT … VALUES` (single/multi-row, Form A/B) via a new zero-width `Node::SetColumn` primitive + an arity-gating tuple lookahead that preserves the §8.1 arity diagnostic; **fully implemented**). **Augments — does NOT supersede — ADR-0030 §4 / ADR-0033 §10**: execution stays verbatim, ADR-0033 Amendment 3's two-command identity (`Insert` vs `SqlInsert`) **stands**. The problem (investigated 2026-05-26; characterization test `sql_insert.rs::sql_dml_skips_app_level_value_validation_that_the_dsl_enforces` proves it): advanced-mode SQL DML gets **none** of the DSL's value feedback — a malformed `date` like `2025/01/15` is silently written, and the offending value is missing from constraint errors — because literal values are spliced into text and discarded (only `STRICT` storage types check them). **Fix (surgical): validate each literal value against its column type before the verbatim insert, and retain it for error reporting — sharing only the per-type validators (`Value::bind_for_column`/`validate_date`/`shortid::validate`), nothing else.** No binding, no statement reconstruction, no auto-fill change, no command-identity collapse — because the two gaps are closed by validation + retention alone, and executing the user's own text is already safe. The literal set = `NULL`/boolean/string/**signed**-numeric; arithmetic/functions/subqueries/column-refs are expressions (skipped — the engine evaluates them). `WHERE` not validated (it's an expression in general; motivation met by `VALUES`/`SET`); `SELECT`/`INSERT … SELECT`/`RETURNING`/`ON CONFLICT` need no special handling since execution is untouched. Phased: **Phase 1** capture-at-parse + validate + retain for `INSERT … VALUES` (no grammar change, no reparse — closes both proven gaps); **Phase 2** `UPDATE … SET` literals; **Phase 3** completion hinting/highlighting (the only part needing a grammar change — a typed-literal slot vs `sql_expr` reusing the DSL `TypedValueSlot`s at `data.rs:141`/`189`/`269`, discriminated by a **boundary-aware lookahead** not a naive `Choice` per **Amendment 1**; split into **3a** `SET` (done) and **3b** `VALUES` (pending); supersedes only Phase 1/2's literal *detection*, not the validation/enrichment on top). Non-goals: binding/reconstruction, collapsing command identity (Am3 stands), changing `serial`/`shortid` auto-fill (`requirements.md` **X4**, a separate possible-bug), a structural `SELECT`, a full SQL-expression AST. Embodies `requirements.md` **X5** (share a *mechanic*, not a *command*); the neutral "that value" safety net (ADR-0035 Amendment 1) stays correct for genuinely-computed values; **Amendment 2 (issue #17, 2026-05-29)** brings the §8.1 arity diagnostic to **simple mode** at parity with advanced: a `tuple_value_list`-style gate (`dsl_insert_value_list`, **simple-mode-gated** so advanced is byte-for-byte unchanged) routes a wrong-count DSL insert tuple to the type-blind fallback so it matches and the friendly arity diagnostic fires (instead of a bare `expected `,`/`)``); `dml_insert_arity_diagnostics` is now mode-aware (advanced Form B = all columns, simple Form B/C = user-fillable since serial/shortid auto-fill, ADR-0018 §3), counts the DSL Form A role (`insert_first_item`) and the keyword-less Form C tuple, with new keys `insert_arity_mismatch_form_b_simple` / `_all_auto`; a wrong-count DSL insert now parses `Ok` + carries the ERROR diagnostic (the `[ERR]` verdict), with a unified Ok-arm submit pre-flight (`dsl_insert_count_mismatch_notes`) blocking dispatch + teaching (the issue #1 Err-arm note retires). **Arity-UX parity only — no consolidation of value-handling/execution/auto-fill; the deliberate mode-distinctness stands** -- [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-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. **Amendment 1 (2026-05-31, issue #10):** the output **tag** is colour-coded by message **status** (its `OutputKind`), not the mode — *narrowing* the side-channel to its stated purpose (the mode tint lives on the **echo** tag alone). `[system]` tag → green, `[error]` tag → red, `Echo` tag → mode tint (the sole exception); **bodies go neutral** (`theme.fg`), the error body **bold** (rustc-style: severity-coloured label, readable bold message — a wall of red prose is harder to read). Yields a status traffic-light (green = ok, red = error) matching the ADR-0040 ✓/✗ markers; supersedes issue #10's three sketched options and closes the tag-colour gap ADR-0040 had flagged as orthogonal - [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/src/ui.rs b/src/ui.rs index e006633..6eead37 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -760,9 +760,20 @@ const fn output_span_style(class: OutputStyleClass, theme: &Theme) -> Style { } fn render_output_line<'a>(line: &'a OutputLine, theme: &Theme) -> Line<'a> { - let tag_style = match line.mode_at_submission { - Mode::Simple => Style::default().fg(theme.mode_simple), - Mode::Advanced => Style::default().fg(theme.mode_advanced), + // ADR-0037 Amendment (issue #10): the output tag is colour-coded by + // the message's STATUS (its kind), not the submission mode — so the + // leftmost glyph the eye lands on says "ok" vs "error" at a glance, + // and a routine `[system]` line never looks identical to an `[error]`. + // The echo line is the sole exception: its tag's whole job is to label + // the submission mode (ADR-0037's stated purpose), so it keeps the + // mode tint (per-command success rides the trailing ✓/✗ — ADR-0040). + let tag_style = match line.kind { + OutputKind::Echo => match line.mode_at_submission { + Mode::Simple => Style::default().fg(theme.mode_simple), + Mode::Advanced => Style::default().fg(theme.mode_advanced), + }, + OutputKind::System | OutputKind::TeachingEcho => Style::default().fg(theme.system), + OutputKind::Error => Style::default().fg(theme.error), }; let tag = match line.kind { OutputKind::Echo => format!("[{}] ", line.mode_at_submission.label().to_lowercase()), @@ -862,14 +873,19 @@ fn render_output_line<'a>(line: &'a OutputLine, theme: &Theme) -> Line<'a> { return Line::from(spans); } + // ADR-0037 Amendment (issue #10): bodies are neutral — the status + // colour lives on the tag, not flooded across the message text. An + // error body keeps weight via BOLD (rustc-style: severity-coloured + // label, readable bold message) rather than a hard-to-read wall of + // red; a system body is plain `theme.fg`. let body_style = match line.kind { - OutputKind::Echo => Style::default().fg(theme.fg), - // TeachingEcho without a prefix falls back to `system` styling - // — the kind itself only signals "render with the dim+lex - // custom path above"; reaching here means the builder produced - // a malformed line, so degrade gracefully rather than crash. - OutputKind::System | OutputKind::TeachingEcho => Style::default().fg(theme.system), - OutputKind::Error => Style::default().fg(theme.error), + // TeachingEcho without a prefix reaches here only as a builder + // bug; it shares the neutral system body so it degrades + // gracefully rather than crashing. + OutputKind::Echo | OutputKind::System | OutputKind::TeachingEcho => { + Style::default().fg(theme.fg) + } + OutputKind::Error => Style::default().fg(theme.fg).add_modifier(Modifier::BOLD), }; Line::from(vec![ Span::styled(tag, tag_style), @@ -1317,10 +1333,11 @@ mod tests { #[test] fn teaching_echo_line_renders_dim_prefix_and_lexed_sql() { // ADR-0038 §4 styled-runs polish: a TeachingEcho line is laid - // out as [tag][dim prefix][lexed SQL spans]. The tag stays the - // mode tint; the `Executing SQL: ` prefix is `theme.muted`; - // the SQL portion is re-lexed in advanced mode so it picks up - // keyword / identifier / literal colours. + // out as [tag][dim prefix][lexed SQL spans]. The tag is the green + // status colour (a `[system]` line — ADR-0037 Amendment, issue + // #10); the `Executing SQL: ` prefix is `theme.muted`; the SQL + // portion is re-lexed in advanced mode so it picks up keyword / + // identifier / literal colours. let theme = Theme::dark(); let line = OutputLine { text: format!( @@ -1429,20 +1446,171 @@ mod tests { } #[test] - fn a_line_without_styled_runs_keeps_whole_line_kind_styling() { + fn system_line_renders_green_tag_and_neutral_body() { + // ADR-0037 Amendment (issue #10): the status-coloured-tag model. + // A `[system]` line's TAG carries the green status colour; its + // BODY is neutral `theme.fg`, not flooded green. The mode tint + // no longer leaks onto system lines (it belongs to the echo line + // alone — ADR-0037's stated purpose). `mode_at_submission` is + // Advanced here precisely to prove the tag is NOT the mode tint. let theme = Theme::dark(); let line = OutputLine { text: "plain system line".to_string(), kind: OutputKind::System, - mode_at_submission: Mode::Simple, + mode_at_submission: Mode::Advanced, styled_runs: None, status: None, }; let rendered = render_output_line(&line, &theme); // tag span + single whole-line body span. assert_eq!(rendered.spans.len(), 2); + assert_eq!(rendered.spans[0].content.as_ref(), "[system] "); + assert_eq!( + rendered.spans[0].style.fg, + Some(theme.system), + "the [system] tag is green (status), not the mode tint", + ); assert_eq!(rendered.spans[1].content.as_ref(), "plain system line"); - assert_eq!(rendered.spans[1].style.fg, Some(theme.system)); + assert_eq!( + rendered.spans[1].style.fg, + Some(theme.fg), + "the system body is neutral, not flooded green", + ); + } + + #[test] + fn error_line_renders_red_tag_and_bold_neutral_body() { + // ADR-0037 Amendment (issue #10): the `[error]` TAG carries the + // red status colour (the leftmost glyph the eye lands on), while + // the BODY renders in neutral `theme.fg` + BOLD (rustc-style: + // severity-coloured label, readable bold message). A wall of red + // prose is hard to read; the red lives on the tag instead. The + // mode tint does not leak onto error lines. + let theme = Theme::dark(); + let line = OutputLine { + text: "no such column: agx".to_string(), + kind: OutputKind::Error, + mode_at_submission: Mode::Advanced, + styled_runs: None, + status: None, + }; + let rendered = render_output_line(&line, &theme); + assert_eq!(rendered.spans.len(), 2); + assert_eq!(rendered.spans[0].content.as_ref(), "[error] "); + assert_eq!( + rendered.spans[0].style.fg, + Some(theme.error), + "the [error] tag is red (status), not the mode tint", + ); + assert_eq!(rendered.spans[1].content.as_ref(), "no such column: agx"); + assert_eq!( + rendered.spans[1].style.fg, + Some(theme.fg), + "the error body is neutral fg, not flooded red", + ); + assert!( + rendered.spans[1].style.add_modifier.contains(Modifier::BOLD), + "the error body is bold for weight without the red-wall readability cost", + ); + } + + #[test] + fn echo_tag_keeps_the_mode_tint_not_a_status_colour() { + // The echo line is the sole exception to the status-tag model: + // its tag's whole job is to label the submission mode (ADR-0037), + // so it keeps the mode tint. Per-command success rides the trailing + // ✓/✗ marker (ADR-0040), not the tag. Locked for both modes so a + // future refactor of `tag_style` cannot regress the echo. + let theme = Theme::dark(); + for (mode, want) in [ + (Mode::Simple, theme.mode_simple), + (Mode::Advanced, theme.mode_advanced), + ] { + let line = OutputLine { + text: format!("{}create table T", crate::dsl::ECHO_PREFIX), + kind: OutputKind::Echo, + mode_at_submission: mode, + styled_runs: None, + status: None, + }; + let rendered = render_output_line(&line, &theme); + assert_eq!( + rendered.spans[0].style.fg, + Some(want), + "echo tag must stay the {mode:?} mode tint", + ); + } + } + + #[test] + fn teaching_echo_tag_is_green_like_other_system_lines() { + // A TeachingEcho is a `[system]`-tagged line, so under the + // status-tag model its tag is green, not the mode tint. The dim + // prefix + lexed-SQL body are unchanged (covered separately). + let theme = Theme::dark(); + let line = OutputLine { + text: format!( + "{}{}", + crate::echo::TEACHING_ECHO_LABEL, + "CREATE TABLE T (id serial PRIMARY KEY)" + ), + kind: OutputKind::TeachingEcho, + mode_at_submission: Mode::Advanced, + styled_runs: None, + status: None, + }; + let rendered = render_output_line(&line, &theme); + assert_eq!(rendered.spans[0].content.as_ref(), "[system] "); + assert_eq!( + rendered.spans[0].style.fg, + Some(theme.system), + "the teaching-echo tag is green (a [system] line), not the mode tint", + ); + } + + #[test] + fn error_and_system_tags_are_distinguishable_in_both_themes() { + // Issue #10 regression guard, stated directly: the `[error]` and + // `[system]` tags must NOT render in the same colour, and neither + // may collapse to the mode tint. Asserted on both palettes — the + // render logic is theme-agnostic, but locking both proves the + // colours themselves stay distinct end to end. + for theme in [Theme::dark(), Theme::light()] { + let tag_fg = |kind| { + render_output_line( + &OutputLine { + text: "x".to_string(), + kind, + mode_at_submission: Mode::Advanced, + styled_runs: None, + status: None, + }, + &theme, + ) + .spans[0] + .style + .fg + }; + let error_tag = tag_fg(OutputKind::Error); + let system_tag = tag_fg(OutputKind::System); + assert_ne!( + error_tag, system_tag, + "[error] and [system] tags must differ ({:?})", + theme.background, + ); + assert_ne!( + error_tag, + Some(theme.mode_advanced), + "the error tag must not be the mode tint ({:?})", + theme.background, + ); + assert_ne!( + system_tag, + Some(theme.mode_advanced), + "the system tag must not be the mode tint ({:?})", + theme.background, + ); + } } fn render_to_string(app: &mut App, theme: &Theme, width: u16, height: u16) -> String {