ADR-0018 implementation: auto-fill contracts for serial and shortid

Generalises serial and shortid beyond their previous restricted forms: - `serial` is no longer restricted to single-column PK. Non-PK serial columns get an emitted UNIQUE constraint and use application-side MAX(col)+1 at INSERT time (rowid alias still drives the PK case for free; per ADR-0010 worker-thread serialisation, the read-then-insert sequence is safe). - `shortid` columns auto-fill existing null cells when the column is materialised — `add column T: x (shortid)` on a non-empty table no longer leaves rows in a not-really-valid NULL state. - `int -> serial` joins the type-change matrix as always-clean identity (closes the asymmetry vs `text -> shortid`); other sources are refused with a route-via-int hint. - `change column T: x (serial|shortid)` fills null source cells with sequence / generated values in the same rebuild transaction. Internal infrastructure: - ReadColumn gains `unique: bool`; read_schema detects single- column UNIQUE indexes via pragma_index_list / pragma_index_info; schema_to_ddl emits inline UNIQUE for non-PK columns. - ColumnSchema (persistence) gains `unique: bool` so the flag survives YAML round-trip and rebuild-from-text reconstructs it faithfully — preserves the "serial -> int leaves UNIQUE in place" promise across save/load cycles. - ChangeColumnTypeResult.client_side now carries `auto_filled` + `auto_fill_kind` alongside `transformed` + `lossy`; the app handler renders separate note lines when both apply. - AddColumnResult is a new return type carrying pre-rendered [client-side] note lines for the auto-fill paths. Tests: 519 -> 534 (+15). Clippy clean.
2026-05-08 14:32:19 +00:00
parent 7dfa718c6e
commit 5bb0a147f0
10 changed files with 1718 additions and 97 deletions
@@ -13,8 +13,8 @@ use tracing::{trace, warn};

 use crate::action::Action;
 use crate::db::{
-    CascadeEffect, ChangeColumnTypeResult, DataResult, DeleteResult, InsertResult,
-    TableDescription, UpdateResult,
+    AddColumnResult, CascadeEffect, ChangeColumnTypeResult, DataResult, DeleteResult,
+    InsertResult, TableDescription, UpdateResult,
 };
 use crate::dsl::{Command, ParseError, parse_command};
 use crate::event::AppEvent;
@@ -317,6 +317,10 @@ impl App {
                self.handle_dsl_change_column_success(&command, result);
                Vec::new()
            }
+            AppEvent::DslAddColumnSucceeded { command, result } => {
+                self.handle_dsl_add_column_success(&command, result);
+                Vec::new()
+            }
            AppEvent::DslFailed { command, error } => {
                self.handle_dsl_failure(&command, &error);
                Vec::new()
@@ -782,6 +786,26 @@ impl App {
        }
    }

+    fn handle_dsl_add_column_success(
+        &mut self,
+        command: &Command,
+        result: AddColumnResult,
+    ) {
+        let summary = format!("[ok] {} {}", command.verb(), command.display_subject());
+        self.note_system(summary);
+        // ADR-0018 §9: emit auto-fill note(s) before the
+        // structure render, so the pedagogical "the tool did
+        // this for you" line is in the user's eye-line next to
+        // the success summary.
+        for note in result.client_side_notes {
+            self.note_system(note);
+        }
+        for line in crate::output_render::render_structure(&result.description) {
+            self.note_system(line);
+        }
+        self.current_table = Some(result.description);
+    }
+
    fn handle_dsl_change_column_success(
        &mut self,
        command: &Command,
@@ -790,28 +814,44 @@ impl App {
        let summary = format!("[ok] {} {}", command.verb(), command.display_subject());
        self.note_system(summary);
        if let Some(note) = result.client_side {
-            // ADR-0017 §6: pedagogical hook telling the learner
-            // "the tool did this for you; raw SQL would need a
-            // CAST or application-level code." Lossy variant
-            // adds the lossy count when --force-conversion was
-            // used.
-            let line = if note.lossy > 0 {
-                format!(
-                    "[client-side] {n} row(s) transformed; {l} of those discarded \
-                     information (lossy). In raw SQL this would need an explicit \
-                     `CAST` or application-level code.",
-                    n = note.transformed,
-                    l = note.lossy
-                )
-            } else {
-                format!(
-                    "[client-side] {n} row(s) were transformed before being stored. \
-                     In raw SQL this would need an explicit `CAST` or \
-                     application-level code.",
-                    n = note.transformed
-                )
-            };
-            self.note_system(line);
+            // ADR-0017 §6 + ADR-0018 §9: pedagogical hook
+            // telling the learner "the tool did this for you;
+            // raw SQL would need explicit CAST / UPDATE / etc."
+            //
+            // When both transformations and auto-fills happen
+            // in the same operation, both note lines are
+            // emitted in order (ADR-0018 §9 explicit rule).
+            if note.transformed > 0 {
+                let line = if note.lossy > 0 {
+                    format!(
+                        "[client-side] {n} row(s) transformed; {l} of those discarded \
+                         information (lossy). In raw SQL this would need an explicit \
+                         `CAST` or application-level code.",
+                        n = note.transformed,
+                        l = note.lossy
+                    )
+                } else {
+                    format!(
+                        "[client-side] {n} row(s) were transformed before being stored. \
+                         In raw SQL this would need an explicit `CAST` or \
+                         application-level code.",
+                        n = note.transformed
+                    )
+                };
+                self.note_system(line);
+            }
+            if note.auto_filled > 0 {
+                let kind = match note.auto_fill_kind {
+                    Some(crate::db::AutoFillKind::Serial) => "serial",
+                    Some(crate::db::AutoFillKind::ShortId) => "shortid",
+                    None => "auto-generated",
+                };
+                self.note_system(format!(
+                    "[client-side] {m} null cell(s) given auto-generated {kind} values. \
+                     In raw SQL this would need an explicit UPDATE to populate.",
+                    m = note.auto_filled,
+                ));
+            }
        }
        for line in crate::output_render::render_structure(&result.description) {
            self.note_system(line);
@@ -8,8 +8,8 @@
 use crossterm::event::KeyEvent;

 use crate::db::{
-    ChangeColumnTypeResult, DataResult, DeleteResult, InsertResult, TableDescription,
-    UpdateResult,
+    AddColumnResult, ChangeColumnTypeResult, DataResult, DeleteResult, InsertResult,
+    TableDescription, UpdateResult,
 };
 use crate::dsl::Command;

@@ -49,6 +49,13 @@ pub enum AppEvent {
        command: Command,
        result: ChangeColumnTypeResult,
    },
+    /// An `add column …` succeeded. `result` carries the
+    /// post-add description plus any `[client-side]` notes
+    /// from the auto-fill paths (ADR-0018 §9).
+    DslAddColumnSucceeded {
+        command: Command,
+        result: AddColumnResult,
+    },
    /// A DSL command failed. `error` is already a friendly
    /// message produced via `DbError::friendly_message`.
    DslFailed {
@@ -360,7 +360,7 @@ mod tests {
    use crate::persistence::ColumnSchema;

    fn col(name: &str, ty: Type) -> ColumnSchema {
-        ColumnSchema { name: name.to_string(), user_type: ty }
+        ColumnSchema { name: name.to_string(), user_type: ty, unique: false }
    }

    #[test]
@@ -100,6 +100,13 @@ pub struct TableSchema {
 pub struct ColumnSchema {
    pub name: String,
    pub user_type: Type,
+    /// Whether this column carries a single-column UNIQUE
+    /// constraint (ADR-0018 §4). Stored explicitly in the
+    /// project YAML so that a `serial → int` round-trip
+    /// (which leaves UNIQUE in place) is preserved across a
+    /// save/load cycle. Defaults to `false` when missing in
+    /// older project files.
+    pub unique: bool,
 }

 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -318,6 +325,7 @@ mod tests {
            columns: vec![ColumnSchema {
                name: "Name".to_string(),
                user_type: Type::Text,
+                unique: false,
            }],
            rows: vec![vec![CellValue::Text("Alice".to_string())]],
        };
@@ -71,12 +71,21 @@ fn write_table(out: &mut String, table: &TableSchema) {
 }

 fn write_column(out: &mut String, col: &ColumnSchema) {
-    let _ = writeln!(
-        out,
-        "      - {{ name: {}, type: {} }}",
-        quote_if_needed(&col.name),
-        col.user_type.keyword(),
-    );
+    if col.unique {
+        let _ = writeln!(
+            out,
+            "      - {{ name: {}, type: {}, unique: true }}",
+            quote_if_needed(&col.name),
+            col.user_type.keyword(),
+        );
+    } else {
+        let _ = writeln!(
+            out,
+            "      - {{ name: {}, type: {} }}",
+            quote_if_needed(&col.name),
+            col.user_type.keyword(),
+        );
+    }
 }

 fn write_relationship(out: &mut String, rel: &RelationshipSchema) {
@@ -181,6 +190,7 @@ pub(crate) fn parse_schema(body: &str) -> Result<SchemaSnapshot, YamlError> {
            columns.push(ColumnSchema {
                name: c.name,
                user_type,
+                unique: c.unique,
            });
        }
        tables.push(TableSchema {
@@ -265,6 +275,11 @@ struct RawColumn {
    name: String,
    #[serde(rename = "type")]
    user_type: String,
+    /// Optional flag introduced in ADR-0018 for single-column
+    /// UNIQUE constraints. Older project files without this
+    /// field default to `false`.
+    #[serde(default)]
+    unique: bool,
 }

 #[derive(Deserialize)]
@@ -295,16 +310,16 @@ mod tests {
                    name: "Customers".to_string(),
                    primary_key: vec!["id".to_string()],
                    columns: vec![
-                        ColumnSchema { name: "id".to_string(), user_type: Type::Serial },
-                        ColumnSchema { name: "Name".to_string(), user_type: Type::Text },
+                        ColumnSchema { name: "id".to_string(), user_type: Type::Serial, unique: false },
+                        ColumnSchema { name: "Name".to_string(), user_type: Type::Text, unique: false },
                    ],
                },
                TableSchema {
                    name: "Orders".to_string(),
                    primary_key: vec!["id".to_string()],
                    columns: vec![
-                        ColumnSchema { name: "id".to_string(), user_type: Type::Serial },
-                        ColumnSchema { name: "CustId".to_string(), user_type: Type::Int },
+                        ColumnSchema { name: "id".to_string(), user_type: Type::Serial, unique: false },
+                        ColumnSchema { name: "CustId".to_string(), user_type: Type::Int, unique: false },
                    ],
                },
            ],
@@ -359,6 +374,7 @@ mod tests {
                columns: vec![ColumnSchema {
                    name: "yes".to_string(),
                    user_type: Type::Bool,
+                    unique: false,
                }],
            }],
            relationships: vec![],
@@ -452,8 +468,8 @@ relationships:
                name: "Items".to_string(),
                primary_key: vec!["a".to_string(), "b".to_string()],
                columns: vec![
-                    ColumnSchema { name: "a".to_string(), user_type: Type::Int },
-                    ColumnSchema { name: "b".to_string(), user_type: Type::Int },
+                    ColumnSchema { name: "a".to_string(), user_type: Type::Int, unique: false },
+                    ColumnSchema { name: "b".to_string(), user_type: Type::Int, unique: false },
                ],
            }],
            relationships: vec![],
@@ -29,8 +29,8 @@ use crate::action::Action;
 use crate::app::App;
 use crate::cli::Args;
 use crate::db::{
-    ChangeColumnTypeResult, DataResult, Database, DbError, DeleteResult, InsertResult,
-    TableDescription, UpdateResult,
+    AddColumnResult, ChangeColumnTypeResult, DataResult, Database, DbError, DeleteResult,
+    InsertResult, TableDescription, UpdateResult,
 };
 use crate::dsl::Command;
 use crate::event::AppEvent;
@@ -951,6 +951,10 @@ fn spawn_dsl_dispatch(
                command: command.clone(),
                result,
            },
+            Ok(CommandOutcome::AddColumn(result)) => AppEvent::DslAddColumnSucceeded {
+                command: command.clone(),
+                result,
+            },
            Err(DbError::PersistenceFatal {
                operation,
                path,
@@ -988,6 +992,7 @@ enum CommandOutcome {
    Update(UpdateResult),
    Delete(DeleteResult),
    ChangeColumn(ChangeColumnTypeResult),
+    AddColumn(AddColumnResult),
 }

 /// Execute a parsed user command and return either a typed
@@ -1016,7 +1021,7 @@ async fn execute_command_typed(
        Command::AddColumn { table, column, ty } => database
            .add_column(table, column, ty, src)
            .await
-            .map(|d| CommandOutcome::Schema(Some(d))),
+            .map(CommandOutcome::AddColumn),
        Command::DropColumn { table, column } => database
            .drop_column(table, column, src)
            .await
@@ -54,13 +54,14 @@ pub fn static_refusal(src: Type, target: Type) -> Option<String> {
    if src == target {
        return Some(format!("column is already `{src}`."));
    }
-    if matches!(target, Type::Serial) {
-        return Some(
-            "the 'serial' type carries auto-increment primary-key semantics \
-             that only apply at create-table time; pick a different target \
-             type (e.g. `int`)."
-                .to_string(),
-        );
+    // ADR-0018 §8: `int → serial` is allowed via the matrix.
+    // Other sources to serial are refused — the user routes
+    // via int first if needed.
+    if matches!(target, Type::Serial) && !matches!(src, Type::Int) {
+        return Some(format!(
+            "to convert from `{src}` to `serial`, change the column to `int` \
+             first; only `int → serial` is supported directly."
+        ));
    }
    if matches!(src, Type::Blob) || matches!(target, Type::Blob) {
        return Some(format!(
@@ -98,6 +99,14 @@ const fn is_in_matrix(src: Type, target: Type) -> bool {
            // stored values"). Storage stays INTEGER; we treat
            // it as an identity transformer.
            | (Serial, Int)
+            // int -> serial: ADR-0018 §8. Storage stays
+            // INTEGER, the metadata flips to "auto-generated"
+            // and the column gains UNIQUE if non-PK. The
+            // matrix entry is identity at the value level;
+            // uniqueness and auto-fill of nulls happen at the
+            // change-column orchestration layer, not in the
+            // per-cell transformer.
+            | (Int, Serial)
            | (Bool, Int | Real | Decimal | Text)
            | (Decimal | Date | DateTime | ShortId | Real, Text)
            // Per-cell-classified
@@ -141,6 +150,16 @@ pub fn transform_cell(src: Type, target: Type, value: &Value) -> CellOutcome {
            Value::Integer(i) => CellOutcome::Clean(Value::Integer(*i)),
            other => unexpected_storage("serial", other),
        },
+        // int -> serial: identity at the storage class level
+        // (both INTEGER); the conversion adds the auto-
+        // generated contract and (for non-PK columns) UNIQUE.
+        // Per-cell transformer is identity; the change-column
+        // orchestrator handles null auto-fill and uniqueness.
+        // ADR-0018 §8.
+        (Int, Serial) => match value {
+            Value::Integer(i) => CellOutcome::Clean(Value::Integer(*i)),
+            other => unexpected_storage("int", other),
+        },

        // ---- Always-clean: bool source (stored as INTEGER 0/1) ----
        (Bool, Int) => match value {
@@ -568,12 +587,29 @@ mod tests {
    }

    #[test]
-    fn anything_to_serial_is_statically_refused() {
+    fn non_int_sources_to_serial_are_statically_refused() {
+        // ADR-0018 §8: only `int → serial` is allowed directly.
+        // Other sources have to route via int. (Same-type
+        // `serial → serial` is the no-op identity refusal.)
        for &src in Type::all() {
+            if matches!(src, Type::Int | Type::Serial) {
+                continue;
+            }
            assert!(static_refusal(src, Type::Serial).is_some(), "{src:?}");
        }
    }

+    #[test]
+    fn int_to_serial_is_allowed() {
+        // ADR-0018 §8: `int → serial` joins the matrix as
+        // always-clean identity. Non-null values pass through.
+        assert!(static_refusal(Type::Int, Type::Serial).is_none());
+        assert_eq!(
+            transform_cell(Type::Int, Type::Serial, &int(42)),
+            CellOutcome::Clean(int(42))
+        );
+    }
+
    #[test]
    fn anything_involving_blob_is_statically_refused() {
        for &other in Type::all() {