docs(adr): /runda DA cleanup — reflect Phases 1-3 done, pin Bucket C
Surfaces from a Devil's-Advocate audit of the DSL → SQL teaching echo (ADR-0038) after Phases 1-3 landed: three doc-drift bugs introduced by the earlier handoff-47 / ADR-promotion commits — requirements.md M4 and both ADR-0038 README index entry + Status block still said "Phase 2 / Phase 3 remain," but `275c726` and `e6ad1ae` shipped them. Updated to reflect actual state: Buckets A + B complete plus the category-3 prose; only the §4 styled-runs polish remains. ADR-0037's README entry also touched to note all four shipping commits of its consumer. Plus a missing test slice the DA flagged: explicit no-echo coverage for the Bucket C cases that flow through command_to_sql's catch-all (show table, explain, replay, every Command::App variant). The contract — ADR-0030 §10 / ADR-0038 §7 Bucket C — forbids echoes for these; a future renderer arm added at the wrong place could silently leak one. The new bucket_c_no_echo_commands_all_return_none pins that. Tests: 2015 passed / 0 failed / 1 ignored (pre-existing); clippy clean. Nothing to escalate.
This commit is contained in:
@@ -2,25 +2,42 @@
|
||||
|
||||
## Status
|
||||
|
||||
Accepted. Design agreed with the user (2026-05-27); **Phase 1 (Bucket A)
|
||||
implemented and verified** by round-tripping every single-statement DDL
|
||||
row + `show data` + the `--all-rows` fall-throughs through the
|
||||
advanced-mode walker (the §1 copy-paste contract). Phase 1 shipped in two
|
||||
commits: handoff-46's `04c8e42` (channel + create-table slice; ADR-0037 +
|
||||
this ADR) and handoff-47's `90479cb` (the renderer's full Bucket A
|
||||
expansion — `Value → SQL-literal` + `Expr → SQL`, `add`/`drop`/`rename`/
|
||||
`change column`, the four `add constraint` + two `drop constraint` forms,
|
||||
`show data [where] [limit]` with PK-sourced `ORDER BY`, and the
|
||||
`delete`/`update … --all-rows` fall-throughs; also fixed a contract gap
|
||||
in the skeleton's create-table echo that silently dropped per-column
|
||||
`DEFAULT` / `CHECK`). **Phase 2 (Bucket B — resolved-name + multi-line
|
||||
echoes), Phase 3 (category-3 prose expansion), and the §4 de-emphasised
|
||||
styled-runs rendering polish remain** — see §8 phasing + handoff-47 §5.
|
||||
**Realises ADR-0030 §10** ("The DSL → SQL teaching bridge") — the Phase-5
|
||||
echo that **ADR-0035 §12 forward-referenced** as "a separate ADR." Builds
|
||||
on **ADR-0037** (the execution-time mode side-channel that gates it) and
|
||||
**ADR-0035 Amendment 2** (the standard-first dialect + `ALTER COLUMN`
|
||||
gap-fill that makes its DDL echoes runnable).
|
||||
Accepted. Design agreed with the user (2026-05-27); **Phases 1–3
|
||||
implemented and verified** by round-tripping every catalogue row
|
||||
(§7 Buckets A, B, plus the §6 category-3 prose) through the
|
||||
advanced-mode walker (the §1 copy-paste contract; §6 category 2
|
||||
holds it per line). Shipped across three feature commits:
|
||||
**Phase 1 — Bucket A single-statement** (handoff-46's `04c8e42`
|
||||
delivered the channel + create-table slice; handoff-47's `90479cb`
|
||||
expanded to the full Bucket A — `Value → SQL-literal` + `Expr → SQL`,
|
||||
`add`/`drop`/`rename`/`change column`, the four `add constraint` +
|
||||
two `drop constraint` forms, `show data [where] [limit]` with
|
||||
PK-sourced `ORDER BY`, and the `delete`/`update … --all-rows`
|
||||
fall-throughs; also fixed a skeleton contract gap that silently
|
||||
dropped per-column `DEFAULT` / `CHECK` on the create-table echo).
|
||||
**Phase 2 — Bucket B resolved-name and multi-statement** (`275c726`):
|
||||
`add index` (auto- and user-named, resolved from the post-execution
|
||||
description), positional `drop index`, `add`/`drop relationship` in
|
||||
both `Endpoints` and `Named` selector forms (the named drop
|
||||
resolved by a small list-tables scan, acceptable for teaching-
|
||||
playground schemas), `drop column --cascade` (multi-line via
|
||||
`DropColumnResult::dropped_indexes`), and `add relationship
|
||||
--create-fk` (multi-line iff the child column was newly created;
|
||||
parent PK type + pre-state captured pre-execution via the runtime's
|
||||
new `collect_echo_lookups`). **Phase 3 — category-3 prose**
|
||||
(`e6ad1ae`): `shortid` generation and type-conversion transforms
|
||||
already surfaced via the pre-existing `client_side.auto_fill_*` /
|
||||
`client_side.transformed*` notes — Phase 3 only added the missing
|
||||
`change column --dont-convert` **caveat** (the only Bucket A
|
||||
caveat; every other category-3 line is illuminating), gated on an
|
||||
advanced effective mode because it references "the line above".
|
||||
**The §4 de-emphasised styled-runs rendering polish remains** — the
|
||||
echo + caveat currently surface as plain `[system]` lines.
|
||||
**Realises ADR-0030 §10** ("The DSL → SQL teaching bridge") — the
|
||||
Phase-5 echo that **ADR-0035 §12 forward-referenced** as "a separate
|
||||
ADR." Builds on **ADR-0037** (the execution-time mode side-channel
|
||||
that gates it) and **ADR-0035 Amendment 2** (the standard-first
|
||||
dialect + `ALTER COLUMN` gap-fill that makes its DDL echoes runnable).
|
||||
|
||||
## Context
|
||||
|
||||
|
||||
+2
-2
File diff suppressed because one or more lines are too long
+16
-11
@@ -132,17 +132,22 @@ handoff-14 cleanup; 449 after B2/C2.)
|
||||
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 progress: **Bucket A complete** (Phase 1 —
|
||||
single-statement DDL + `show data` + `--all-rows` fall-throughs,
|
||||
every row round-tripped per ADR-0038 §1; handoff-47);
|
||||
**Bucket B** (resolved-name + multi-line echoes — `add index`,
|
||||
relationship add/drop, `drop column --cascade`,
|
||||
`add relationship --create-fk`) and **category-3 prose**
|
||||
(`shortid` generation, type-conversion transforms,
|
||||
`change column --dont-convert` caveat) are deferred to Phase
|
||||
2 / Phase 3 per ADR-0038 §8. The de-emphasised styled-runs
|
||||
rendering polish (ADR-0038 §4 / ADR-0028) is also still pending —
|
||||
the echo currently surfaces as a plain `[system]` line.
|
||||
silent. Echo coverage: **Buckets A and B complete, plus
|
||||
category-3 prose** — every catalogue row in ADR-0038 §7
|
||||
round-trips per line through the advanced walker (the §1
|
||||
copy-paste contract; §6 category 2 holds it per line). Shipped
|
||||
across three commits: **Phase 1** Bucket A — single-statement DDL
|
||||
+ `show data` + `--all-rows` fall-throughs (`90479cb`); **Phase 2**
|
||||
Bucket B — resolved-name + multi-line echoes (`add index` auto- and
|
||||
user-named, positional `drop index`, `add`/`drop relationship` in
|
||||
both selector forms, `drop column --cascade`, `add relationship
|
||||
--create-fk`) (`275c726`); **Phase 3** the category-3 prose —
|
||||
`shortid` generation and type-conversion transforms via the
|
||||
pre-existing `client_side.auto_fill_*` / `client_side.transformed*`
|
||||
notes, plus the new `change column --dont-convert` caveat
|
||||
(`e6ad1ae`). The only remaining ADR-0038 item is the §4
|
||||
de-emphasised **styled-runs rendering polish** (ADR-0028) — the
|
||||
echo + caveat currently surface as plain `[system]` lines.
|
||||
|
||||
## App-level commands (per ADR-0003)
|
||||
|
||||
|
||||
+74
@@ -1118,6 +1118,80 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bucket_c_no_echo_commands_all_return_none() {
|
||||
// ADR-0038 §7 Bucket C: pin the no-echo cases that flow through
|
||||
// the `command_to_sql` catch-all (alongside the already-tested
|
||||
// `Sql*`/`Select` and column-level UNIQUE/CHECK drop). A drift
|
||||
// here — say, a renderer arm grown into a Bucket C command —
|
||||
// would silently leak echoes the §10 / §13 contract forbids.
|
||||
use crate::dsl::command::AppCommand;
|
||||
|
||||
// `show table T` — structure display, no SQL spelling in the
|
||||
// surface (ADR-0038 §7 Bucket C).
|
||||
assert!(
|
||||
command_to_sql(&Command::ShowTable {
|
||||
name: "T".to_string(),
|
||||
})
|
||||
.is_none(),
|
||||
"show table is Bucket C — no echo"
|
||||
);
|
||||
|
||||
// `explain …` — EXPLAIN of advanced SQL is the deferred OOS-2
|
||||
// follow-up (ADR-0039); the DSL `explain` wrapper itself echoes
|
||||
// nothing (ADR-0038 §7).
|
||||
assert!(
|
||||
command_to_sql(&Command::Explain {
|
||||
query: Box::new(Command::ShowData {
|
||||
name: "T".to_string(),
|
||||
filter: None,
|
||||
limit: None,
|
||||
}),
|
||||
})
|
||||
.is_none(),
|
||||
"explain is Bucket C — no echo"
|
||||
);
|
||||
|
||||
// `replay <path>` — app-lifecycle, no SQL form (ADR-0038 §7).
|
||||
// (Also: replay bypasses the echo-bearing spawn entirely, so it
|
||||
// never reaches command_to_sql in practice — but pinning the
|
||||
// catch-all here guards against a future arm sneaking in.)
|
||||
assert!(
|
||||
command_to_sql(&Command::Replay {
|
||||
path: "history.log".to_string(),
|
||||
})
|
||||
.is_none(),
|
||||
"replay is Bucket C — no echo"
|
||||
);
|
||||
|
||||
// `Command::App(_)` — every app-lifecycle command, regardless of
|
||||
// verb. ADR-0030 §10: "app-level commands have no SQL form and
|
||||
// are not echoed." Sample two: `quit` (verb-only) and `mode
|
||||
// advanced` (verb + payload).
|
||||
for app in [
|
||||
AppCommand::Quit,
|
||||
AppCommand::Help,
|
||||
AppCommand::Rebuild,
|
||||
AppCommand::Save,
|
||||
AppCommand::New,
|
||||
AppCommand::Load,
|
||||
AppCommand::Undo,
|
||||
AppCommand::Redo,
|
||||
AppCommand::Mode {
|
||||
value: crate::dsl::command::ModeValue::Advanced,
|
||||
},
|
||||
] {
|
||||
assert!(
|
||||
command_to_sql(&Command::App(app.clone())).is_none(),
|
||||
"Command::App({app:?}) is Bucket C — no echo"
|
||||
);
|
||||
// Also confirm echo_for gates the same in advanced mode.
|
||||
assert!(
|
||||
echo_for(&Command::App(app), EffectiveMode::AdvancedPersistent).is_none(),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sql_entered_command_is_not_echoed() {
|
||||
// A command the user typed as SQL (SqlCreateTable) is not echoed
|
||||
|
||||
Reference in New Issue
Block a user