- F2-broad: replay failures now render with real schema context instead of
a contextless friendly_message(). Extract App::build_translate_context into
the shared App::translate_context_for(command, facts, verbosity); run_replay
enriches via enrich_dsl_failure + that builder. ctx_* fallbacks degrade to
neutral prose so the rare non-replay contextless callsites can't leak raw
{name} either. (SQL INSERT/UPDATE values aren't retained — ADR-0033 verbatim
— so those show real table/column + neutral "that value".)
- Gap C: SQL ALTER … ADD FOREIGN KEY on a missing child column refuses with an
SQL-appropriate "add it first", not the DSL-only --create-fk flag.
- Gap B: dropping a single-column-UNIQUE column refuses with a pointer to
`drop constraint unique from T.col` (was an opaque generic refusal).
- Gap D: 4e drop/rename CHECK-guard + 4f change-type FK-guard refusals reworded
to explain why; static_refusal reasons left as-is.
Tests: +4, 3 strengthened. 1926 pass / 0 fail / 0 skip; clippy clean.
10 KiB
Plan — ADR-0035 Phase-4 /runda follow-ups F1 / F2 / F3 (2026-05-26)
Bundle of the three error-message / capability follow-ups surfaced by the
whole-Phase-4 /runda (handoff-42 §3). All three live on the safe
composite-UNIQUE edge (dropping a UNIQUE-covered column is correctly
refused today — no corruption); the work improves messaging and adds a
way to drop the constraint itself.
Phase 1 — requirements
-
F1 — friendly refusal for dropping a composite-UNIQUE column.
do_drop_column's covering-index guard readsread_table_indexes, which filters toorigin='c'(explicitCREATE INDEX) and excludes the UNIQUE-constraint auto-index (origin='u'). Sodrop column cwhenunique (b, c)spanscskips the guard, reaches the engine, and is refused with an unhelpful generic message. Add an up-front guard detecting the column inschema.unique_constraints(composite only —read_unique_constraintsroutes single-column UNIQUEs to the column flag, multi-column tounique_constraints), refusing with the constraint's derived name (F3) + the drop command. Behaviour stays "refused"; only the message improves. Message-only — no--cascadeextension (the SQL drop-column has no--cascadespelling; dropping a constraint via cascade is a larger semantic change, out of scope unless the user asks). -
F2 — literal
{table}leak in contextlessfriendly_message().Verbositydefaults toVerbose, sofriendly_message()(which usesTranslateContext::default(), no table) renders the generic hint"…current state of{table}."with the literal placeholder viactx_table()'s"{table}"fallback. Hits every contextlessfriendly_message()callsite whose error lands in the generic bucket: replay, undo, rebuild-from-text, export. Fix: a tableless generic-hint variant selected whenctx.tableisNone. Broader finding (DA): the same{name}-marker fallbacks leak in other templates (e.g. a replayed UNIQUE violation →error.unique.*) when reached contextless. The documented F2 is the generic case; the broader leak is surfaced for the user to scope, not silently expanded/narrowed. -
F3 — a way to drop an anonymous composite UNIQUE (user-raised). By design (§4a.2/§4g) a composite
UNIQUE(a,b)is anonymous — PRAGMA-detected, a bare column-list, no name — soDROP CONSTRAINT <name>can't target it and recreating the table is the only escape. Add a way to drop it. (Amends ADR-0035 — see Amendment 1.)
Baseline: cargo test → 1917 pass / 0 fail / 0 skip / 1 ignored doctest;
cargo clippy --all-targets -- -D warnings clean.
Phase 2/3 — F3 design (the genuine fork; user-decided)
Composite UNIQUE has no name. Options considered:
- A — name composite UNIQUEs (user-supplied): reverse the §4g
anonymity decision; needs a new
__rdbms_*table + YAML round-trip + rebuild-arrival migration (the cost §4a.3 deliberately avoided). Most SQL-standard, largest. - B — positional drop by column-list (
drop … unique (cols)): preserves anonymity, no metadata, but needs a new grammar form. - C — auto-assigned, engine-neutral derived name (chosen). The
name is a deterministic function of the columns (
unique_<cols>), recomputed live wherever shown or matched. Storage stays a bare column-list (anonymity preserved); the name is purely a presentation/addressing label. Reuses the existingDROP CONSTRAINT <name>grammar — no new syntax at all. Zero metadata, zero migration, round-trips for free. Tracks column renames.
User decisions (2026-05-26): approach C / derived (no storage);
name format unique_<cols>; doc vehicle amend ADR-0035; scope
advanced-SQL only (matching the 4g ADD form — no simple-mode verb).
DA critique (written down)
- Ambiguous derived names (e.g. a column literally named
b_cvsUNIQUE (b, c)): drop-by-name must detect ambiguity and refuse, never guess. In scope. - Collision with a user-named CHECK/FK of the same string: the
do_drop_constraint_by_nameorder is CHECK → FK → UNIQUE, so a CHECK/FK shadows a derived UNIQUE name. Acceptable given the distinctiveunique_prefix; document the order. - F1
--cascade: not extended to drop a covering UNIQUE (constraint, not index). Refuse-only. Flagged. - F2 breadth: the leak is broader than
error.generic.hint. Fix the documented generic case; surface the broader leak. Flagged. - Single-column UNIQUE column drop: a parallel gap (a single-column
UNIQUE column drop also reaches the engine with a poor message) exists
but is outside the documented F1 scope (different mechanism —
ADR-0029 column-level
drop constraint). Noted, not fixed here.
Phase 4 — execution (order: F3 → F1 → F2)
- F3.
unique_constraint_name(cols) -> "unique_<cols>"helper (db.rs,pub(crate)). Extenddo_drop_constraint_by_namewith a third step: match each composite UNIQUE's derived name; >1 match → refuse (ambiguous); 1 match →rebuild_tablewith that entry removed fromunique_constraints(mirrorsdo_alter_add_uniquein reverse) +do_describe_table. Annotate the describe "Table constraints:" section:unique_b_c: UNIQUE (b, c). - F1. Up-front guard in
do_drop_columnafter the index-covering guard: column in anyschema.unique_constraintsentry → refuse with the derived name +alter table T drop constraint <name>. - F2.
error.generic.hint_no_tablecatalog entry; intranslate_generic, pick it whenctx.tableisNone.
Test-first for each (reproduce → fail → fix → pass), across the worker API (Tier-1/3) and the friendly-layer unit tests + insta snapshots.
Phase 5 — verification
Full cargo test + clippy; compare to baseline; every checklist item
addressed; engine-neutral vocab held (no SQLite/STRICT/PRAGMA in new
user-facing strings); ADR + README + this plan lockstep.
Shipped 2026-05-26 as commit cb8ff8a — 1922 pass / 0 fail / 0 skip,
clippy clean.
Follow-up (2026-05-26, user-approved) — broad F2 + message gaps B/C/D
After the F1–F3 commit the user asked to take the broader F2 leak plus the remaining message gaps. Scope (user-decided):
- F2-broad — enrich replay + neutral-prose safety net. The constraint
templates (
error.unique.*,error.foreign_key.*,error.check.*) carry{table}/{column}/{value}in the headline, so they leak whenever rendered via contextlessfriendly_message(). The realistic surface is replay of a constraint-violating scripted command (run_replay's failure branch,runtime.rs, calls baree.friendly_message()). Fix: (a) replay reusesenrich_dsl_failure+ the operation-from-Commandmapping so a replayed failure shows the real table/column/value (best UX); (b) thectx_*fallback markers become neutral prose ({table}→ "the table", etc.) so the rare non-replay contextless callsites (undo/rebuild/export) can't leak raw{name}either. Requires extractingApp::build_translate_contextinto apub(crate)free fn (parameterised by verbosity) so replay and the App share one Command→context mapping. - Gap C —
--create-fkleak. SQLALTER … ADD FOREIGN KEYon a missing child column reusesdo_add_relationship's DSL-flavoured error suggesting--create-fk(a DSL flag, meaningless in SQL). Fix:do_alter_add_foreign_keypre-validates the child column and emits an SQL-appropriate "add it first" refusal with no flag mention. - Gap B — single-column UNIQUE column drop. Parallel to F1 but a
different mechanism: a single-column UNIQUE rides on the column
uniqueflag (ADR-0029), notunique_constraints. Characterise current behaviour with a test, then add a friendly, actionable refusal pointing at the column-leveldrop constraint unique from T.col. - Gap D — terse CHECK-guard / type-conversion wording. Polish the 4e drop/rename-column CHECK-guard refusals and the 4f type-conversion diagnostics for clarity, staying engine-neutral. Conservative — wording only, no behaviour change.
DA critique (follow-up)
- Refactor risk. Extracting
build_translate_contextfromAppis a pure move + signature change (addverbosity); the App method becomes a thin delegator. Covered by the existing app tests + a new replay render test. ctx_*neutral prose looks odd backtick-wrapped (`the table`) — accepted by the user as a last-resort safety net; it renders only in the near-impossible non-replay constraint case (replay is enriched).- Gap B may be a non-issue if the engine drops a single-column-UNIQUE column cleanly — characterise first, only guard if it refuses.
- No marker pinned anywhere. No test/snapshot asserts a literal
{table}/{column}as expected output, so changing the fallbacks is low-risk (verified by grep).
Outcome (implemented 2026-05-26)
- F2-broad —
App::build_translate_contextextracted to the sharedApp::translate_context_for(command, facts, verbosity);run_replay's failure branch now enriches viaenrich_dsl_failure+ that builder, so a replayed failure shows the real table/column (and value/parent/rule where resolvable).ctx_*fallbacks are neutral prose. Discovered limitation: replay parses in advanced mode → SQLINSERT/UPDATE, whose values are raw SQL text (ADR-0033 verbatim), not retained — so the offending value degrades to "that value" (no leak), while table/column are real. DSLinsert/updatestill show the value. (Same gap exists on the interactive SQL-DML path; the safety net covers it.) - Gap C —
do_alter_add_foreign_keypre-validates the child column and emits an SQL-appropriate "add it first" refusal (no--create-fk). - Gap B —
do_drop_columnguards a single-column UNIQUE (col_info.unique) with a refusal pointing atdrop constraint unique from T.col. - Gap D — polished the 4e drop/rename CHECK-guard refusals and the 4f
change-type FK guard to explain why; left
static_refusalreasons as-is (already clear — avoided gratuitous churn).
Tests +4 (replay no-leak, safety-net unit, FK-missing-column, single-col UNIQUE drop) + 3 strengthened (2× CHECK-guard wording, 1× change-type FK wording). 1926 pass / 0 fail / 0 skip, clippy clean.