From 431645ae60404f8382800c64d69899183917e9ac Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Sat, 9 May 2026 22:10:05 +0000 Subject: [PATCH] =?UTF-8?q?ADR-0019=20=C2=A76:=20runtime=20enrichment=20+?= =?UTF-8?q?=20row=20pinpointing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the placeholder-substitution gap reported during manual testing: FK violations were rendering `` and `` literally because the App had no schema awareness. With this change the runtime resolves the schema-dependent facts before the App ever sees the failure. ## Architecture - **Database** gains two public methods backed by new worker Request variants: - `read_relationships(table)` → (outbound, inbound) FK list (lifts the previously-private `read_relationships_*` pair into the public surface, behind a `RelationshipsReply` type alias). - `find_rows_matching(table, column, value, limit)` → `DataResult` for row pinpoint queries. - **friendly module** gets: - New `FailureContext` struct: schema-resolved facts the runtime builds (table, column, value, parent_table, parent_column, child_table, optional diagnostic_table). - `TranslateContext` loses its lifetime parameter and gains `parent_table` / `parent_column` fields. All string fields are now `Option` for ownership simplicity. - `TranslateContext::from_facts(operation, verbosity, facts)` helper. - Translator's FK paths now use `ctx.parent_table` / `ctx.parent_column` for child-side wording; FK Update gets a dedicated `fk_child_side_update` arm. - FK dispatch is enrichment-driven first (`parent_table` set → child-side; `child_table` set → parent-side), with operation as the tiebreaker. - The translator forwards `ctx.diagnostic_table` onto the `FriendlyError` so pinpointed rows render through the existing ADR-0017 §7 bordered renderer. - **Event** `DslFailed` carries `(command, error, facts)`. The runtime populates `facts` via `enrich_dsl_failure` before posting the event. - **Runtime** `enrich_dsl_failure(database, command, error)` classifies and resolves: - UNIQUE INSERT/UPDATE: parses `T.col` from engine message, finds the user's attempted value (with schema fallback for natural-order multi-value INSERT — including the serial/shortid auto-skip rule from `do_insert`), pinpoints the existing conflicting row(s) via `find_rows_matching` and renders as a `DiagnosticTable`. - NOT NULL INSERT/UPDATE: parses `T.col`; no value (definitionally null) and no pinpoint (engine doesn't identify the row). - FK INSERT/UPDATE: outbound relationship lookup picks the FK column the user is touching; resolves `parent_table`/`parent_column`/`value`. UPDATE falls back to inbound (parent-side) when no outbound match. - FK DELETE: inbound relationship lookup picks a child_table that references this row. - **App** drops its old `attempted_value_for` / `column_from_qualified_target` helpers (their work moved to runtime where the Database is in scope). `build_translate_context` combines the runtime-supplied facts with the operation derived from the Command and the App's verbosity. ## Manual-test fixes folded in Two issues surfaced during manual testing of the initial implementation, both fixed: 1. Natural-order multi-value INSERT (`insert into Orders values (4, 11.99)`) skipped FK enrichment because `user_value_for_column` only knew the single-value short form. The schema-aware lookup (`user_value_for_column_with_schema`) now mirrors `do_insert`'s position-mapping rule (auto-generated columns skipped), so positional INSERTs onto tables with serial/shortid PKs resolve correctly. Regression test: `enrich_fk_insert_natural_order_multi_value_resolves_via_schema`. 2. The arity error on INSERT now lists the columns it expected — `expected 3 value(s) for (id, Name, Email), got 2` instead of the bare count. Surfaces what the user needs to fix without making them go check the schema. ## Tests `tests/friendly_enrichment.rs` (+8 integration tests): - UNIQUE INSERT with explicit columns: facts.{table, column, value, diagnostic_table} all resolved; pinpoint shows conflicting row. - UNIQUE INSERT natural-order short form: schema fallback resolves the value. - UNIQUE UPDATE: value pulled from assignments. - NOT NULL INSERT: table+column resolved, value None (correct), no pinpoint. - FK INSERT: parent_table, parent_column, value all resolved via outbound relationship lookup. - FK INSERT natural-order multi-value: schema-aware lookup with auto-skip resolves correctly (regression for the manual-test bug). - FK DELETE: child_table resolved via inbound relationship lookup. - DbError::Unsupported: enrichment returns default FailureContext (no false positives). App-level tests updated to populate `FailureContext` directly (simulating runtime enrichment) for the verbosity / threading checks. ## Tally 610 tests passing (was 603: +8 enrichment integration tests minus 1 obsolete App-side helper test that the runtime absorbed). Clippy clean with nursery lints. Release builds. --- src/app.rs | 227 ++++++---------- src/db.rs | 168 +++++++++++- src/event.rs | 9 +- src/friendly/mod.rs | 2 +- src/friendly/translate.rs | 241 ++++++++++++----- src/runtime.rs | 295 +++++++++++++++++++- tests/friendly_enrichment.rs | 506 +++++++++++++++++++++++++++++++++++ tests/walking_skeleton.rs | 1 + 8 files changed, 1231 insertions(+), 218 deletions(-) create mode 100644 tests/friendly_enrichment.rs diff --git a/src/app.rs b/src/app.rs index 00b47fd..ada3396 100644 --- a/src/app.rs +++ b/src/app.rs @@ -326,8 +326,12 @@ impl App { self.handle_dsl_add_column_success(&command, result); Vec::new() } - AppEvent::DslFailed { command, error } => { - self.handle_dsl_failure(&command, error); + AppEvent::DslFailed { + command, + error, + facts, + } => { + self.handle_dsl_failure(&command, error, facts); Vec::new() } AppEvent::TablesRefreshed(tables) => { @@ -932,11 +936,19 @@ impl App { } } - fn handle_dsl_failure(&mut self, command: &Command, error: crate::db::DbError) { + fn handle_dsl_failure( + &mut self, + command: &Command, + error: crate::db::DbError, + facts: crate::friendly::FailureContext, + ) { // Render through the friendly-error layer (ADR-0019). // The translator picks operation-tailored wording from // the catalog and applies the user's current verbosity. - let ctx = self.build_translate_context(command, &error); + // `facts` carries schema-resolved enrichment (parent + // tables, attempted values, pinpointed rows) the + // runtime built before posting the event. + let ctx = self.build_translate_context(command, facts); let rendered = crate::friendly::translate_error(&error, &ctx).render(); warn!( verb = command.verb(), @@ -956,24 +968,24 @@ impl App { )); } - /// Construct a [`TranslateContext`] from a [`Command`] and - /// the App's current verbosity. Drives the operation-tailored - /// wording the catalog produces (ADR-0019 §4). + /// Construct a [`TranslateContext`] by combining the + /// runtime-supplied [`FailureContext`] (schema-resolved + /// facts) with the operation derived from the originating + /// [`Command`] and the App's current verbosity. /// - /// `error` is consulted to extract the attempted value where - /// the engine's constraint message names a column — for - /// UNIQUE / NOT NULL on INSERT the user's value flows through - /// to the `{value}` placeholder so the headline reads - /// "...already has the value `5`" rather than "...has the - /// value ``". - fn build_translate_context<'a>( + /// Schema-resolved facts win over Command-derived + /// fallbacks where the runtime supplied them — typically + /// the runtime knows more (the FK-relationship lookup + /// produces `parent_table` that the Command alone can't + /// reveal). + fn build_translate_context( &self, - command: &'a Command, - error: &crate::db::DbError, - ) -> crate::friendly::TranslateContext<'a> { + command: &Command, + facts: crate::friendly::FailureContext, + ) -> crate::friendly::TranslateContext { use crate::dsl::{Command as C, RelationshipSelector}; use crate::friendly::{Operation, TranslateContext}; - let (operation, table, column) = match command { + let (operation, fallback_table, fallback_column) = match command { C::CreateTable { name, .. } => (Operation::CreateTable, Some(name.as_str()), None), C::DropTable { name } => (Operation::DropTable, Some(name.as_str()), None), C::AddColumn { table, column, .. } => ( @@ -1022,26 +1034,22 @@ impl App { C::Replay { .. } => (Operation::Replay, None, None), }; - // Try to find the value the user attempted on the - // offending column so the catalog `{value}` placeholder - // gets a concrete substitution. Best-effort: works when - // the engine reports a qualified `T.col` AND the - // command supplies explicit columns (or has a - // single-value short form). Multi-value natural-order - // INSERTs hide which positional value belongs to which - // column without a schema lookup — the translator's - // fallback `` reads sensibly there until the - // runtime-side row-pinpoint follow-on (ADR-0019 §6) - // adds schema awareness. - let value = attempted_value_for(command, error); - TranslateContext { operation: Some(operation), - table, - column, - value, + table: facts + .table + .or_else(|| fallback_table.map(str::to_string)), + column: facts + .column + .or_else(|| fallback_column.map(str::to_string)), + child_table: facts.child_table, + parent_table: facts.parent_table, + parent_column: facts.parent_column, + src_type: None, + target_type: None, + value: facts.value, + diagnostic_table: facts.diagnostic_table, verbosity: self.messages_verbosity, - ..TranslateContext::default() } } @@ -1503,73 +1511,6 @@ impl App { } } -/// Best-effort extraction of the user's attempted value for the -/// column an engine constraint error implicates. Used to fill -/// the catalog's `{value}` placeholder so the headline reads -/// "...already has the value `5`" rather than ``. -/// -/// Returns `None` when: -/// - the error isn't a `Sqlite { … }` payload (no engine -/// message to parse the column from); -/// - the engine message doesn't carry a qualified `T.col` -/// target (e.g. raw "FOREIGN KEY constraint failed"); -/// - the command isn't an INSERT/UPDATE (DELETE has no value -/// to attribute); -/// - the command supplies multiple natural-order values and -/// we can't tell which slot belongs to the named column -/// without a schema lookup. -fn attempted_value_for( - command: &Command, - error: &crate::db::DbError, -) -> Option { - use crate::dsl::Command as C; - let crate::db::DbError::Sqlite { message, .. } = error else { - return None; - }; - let column = column_from_qualified_target(message)?; - match command { - C::Insert { - columns, values, .. - } => { - if let Some(cols) = columns { - let idx = cols.iter().position(|c| c == &column)?; - values.get(idx).map(ToString::to_string) - } else if values.len() == 1 { - // Natural-order short form `insert into T (v)` — - // a single value can only belong to a single - // column. The schema would still need to confirm - // the column name matches, but for the common - // single-column-table case the value is right. - values.first().map(ToString::to_string) - } else { - None - } - } - C::Update { assignments, .. } => assignments - .iter() - .find(|(c, _)| c == &column) - .map(|(_, v)| v.to_string()), - _ => None, - } -} - -/// Pull the column name out of a qualified target like -/// `"UNIQUE constraint failed: T.col"`. Used by -/// `attempted_value_for`. Returns `None` when the message -/// doesn't have the expected shape. -fn column_from_qualified_target(message: &str) -> Option { - let after = message.split_once(':')?.1.trim(); - let first = after.split(',').next()?.trim(); - let mut parts = first.splitn(2, '.'); - let _table = parts.next()?; - let column = parts.next()?.trim(); - if column.is_empty() { - None - } else { - Some(column.to_string()) - } -} - fn parse_error_message(err: &ParseError) -> String { match err { ParseError::Invalid { message, .. } => message.clone(), @@ -1969,6 +1910,7 @@ mod tests { message: "no such table: Ghost".to_string(), kind: crate::db::SqliteErrorKind::NoSuchTable, }, + facts: crate::friendly::FailureContext::default(), }); let last = app.output.back().unwrap(); assert_eq!(last.kind, OutputKind::Error); @@ -2000,10 +1942,12 @@ mod tests { } #[test] - fn dsl_failure_substitutes_attempted_value_for_unique_insert() { - // The user's reported case: `insert into thing (1)` - // (single-value short form) hits a UNIQUE constraint; - // the headline should show `1` rather than ``. + fn dsl_failure_threads_facts_value_into_unique_insert_headline() { + // The runtime resolves the user's attempted value into + // `FailureContext::value` (Phase C). The App threads + // it into `TranslateContext.value` and the catalog + // headline gets the concrete substitution. Here we + // simulate the runtime by populating `facts` directly. let mut app = App::new(); let cmd = Command::Insert { table: "thing".to_string(), @@ -2014,7 +1958,17 @@ mod tests { message: "UNIQUE constraint failed: thing.id".to_string(), kind: crate::db::SqliteErrorKind::UniqueViolation, }; - app.update(AppEvent::DslFailed { command: cmd, error: err }); + let facts = crate::friendly::FailureContext { + table: Some("thing".to_string()), + column: Some("id".to_string()), + value: Some("1".to_string()), + ..crate::friendly::FailureContext::default() + }; + app.update(AppEvent::DslFailed { + command: cmd, + error: err, + facts, + }); let body = app .output .iter() @@ -2026,49 +1980,16 @@ mod tests { "expected the attempted value `1` in headline:\n{body}" ); assert!( - !body.contains(""), - " placeholder should have been substituted:\n{body}" + !body.contains("{value}"), + "{{value}} placeholder should have been substituted:\n{body}" ); } #[test] - fn dsl_failure_substitutes_attempted_value_for_unique_with_explicit_columns() { - // Columns explicitly listed: the helper should match - // the failing column to its position in the column list - // and substitute the corresponding value. - let mut app = App::new(); - let cmd = Command::Insert { - table: "Customers".to_string(), - columns: Some(vec![ - "name".to_string(), - "id".to_string(), - ]), - values: vec![ - crate::dsl::Value::Text("Alice".to_string()), - crate::dsl::Value::Number("42".to_string()), - ], - }; - let err = crate::db::DbError::Sqlite { - message: "UNIQUE constraint failed: Customers.id".to_string(), - kind: crate::db::SqliteErrorKind::UniqueViolation, - }; - app.update(AppEvent::DslFailed { command: cmd, error: err }); - let body = app - .output - .iter() - .map(|l| l.text.as_str()) - .collect::>() - .join("\n"); - assert!( - body.contains("`42`"), - "expected attempted id `42`:\n{body}" - ); - assert!(!body.contains("`Alice`"), "wrong column substituted:\n{body}"); - } - - #[test] - fn dsl_failure_substitutes_attempted_value_for_unique_update() { - // UPDATE: the value comes from the SET assignment list. + fn dsl_failure_threads_facts_value_into_unique_update_headline() { + // UPDATE: same threading as INSERT, just that the + // runtime would have pulled `value` from the SET + // assignment matching the offending column. let mut app = App::new(); let cmd = Command::Update { table: "Customers".to_string(), @@ -2085,7 +2006,17 @@ mod tests { message: "UNIQUE constraint failed: Customers.id".to_string(), kind: crate::db::SqliteErrorKind::UniqueViolation, }; - app.update(AppEvent::DslFailed { command: cmd, error: err }); + let facts = crate::friendly::FailureContext { + table: Some("Customers".to_string()), + column: Some("id".to_string()), + value: Some("7".to_string()), + ..crate::friendly::FailureContext::default() + }; + app.update(AppEvent::DslFailed { + command: cmd, + error: err, + facts, + }); let body = app .output .iter() @@ -2115,6 +2046,7 @@ mod tests { app.update(AppEvent::DslFailed { command: cmd.clone(), error: err(), + facts: crate::friendly::FailureContext::default(), }); let verbose_text = app .output @@ -2133,6 +2065,7 @@ mod tests { app.update(AppEvent::DslFailed { command: cmd, error: err(), + facts: crate::friendly::FailureContext::default(), }); let short_text = app .output diff --git a/src/db.rs b/src/db.rs index f989c0b..f7d9753 100644 --- a/src/db.rs +++ b/src/db.rs @@ -72,6 +72,11 @@ pub struct TableDescription { /// Used for both outbound (this table is the child, holding the /// FK column) and inbound (this table is the parent being /// referenced) sides; the field meanings flip per side. +/// `(outbound, inbound)` pair returned by +/// [`Database::read_relationships`]. +pub type RelationshipsReply = + Result<(Vec, Vec), DbError>; + #[derive(Debug, Clone, PartialEq, Eq)] pub struct RelationshipEnd { /// User-facing name of the relationship (auto-generated or @@ -428,6 +433,28 @@ enum Request { source: Option, reply: oneshot::Sender>, }, + /// Read both directions of FK relationships for `table`. + /// Returns `(outbound, inbound)` — outbound = rows where + /// `table` is the child (has FK columns pointing elsewhere); + /// inbound = rows where `table` is the parent (referenced + /// by other tables). Used by the friendly-error layer's + /// runtime enrichment (ADR-0019 §6). + ReadRelationships { + table: String, + reply: oneshot::Sender, + }, + /// Find rows in `table` where `column` matches `value`. + /// Capped at `limit` rows. Used by the friendly-error + /// layer's row-pinpoint diagnostic (ADR-0019 §6, ADR-0017 §7). + /// Best-effort: returns empty rows on any failure (no row + /// matched, schema gone, type mismatch on bind, etc.). + FindRowsMatching { + table: String, + column: String, + value: Value, + limit: usize, + reply: oneshot::Sender>, + }, } impl Database { @@ -737,6 +764,41 @@ impl Database { recv.await.map_err(|_| DbError::WorkerGone)? } + /// Read both directions of FK relationships for `table`. + /// Used by the runtime's friendly-error enrichment to + /// resolve parent / child table names (ADR-0019 §6). + pub async fn read_relationships( + &self, + table: String, + ) -> RelationshipsReply { + let (reply, recv) = oneshot::channel(); + self.send(Request::ReadRelationships { table, reply }).await?; + recv.await.map_err(|_| DbError::WorkerGone)? + } + + /// Pinpoint rows in `table` where `column` matches `value`. + /// Used by the runtime's friendly-error enrichment to + /// surface offending rows after a UNIQUE / FK violation + /// (ADR-0019 §6, ADR-0017 §7). Capped at `limit`. + pub async fn find_rows_matching( + &self, + table: String, + column: String, + value: Value, + limit: usize, + ) -> Result { + let (reply, recv) = oneshot::channel(); + self.send(Request::FindRowsMatching { + table, + column, + value, + limit, + reply, + }) + .await?; + recv.await.map_err(|_| DbError::WorkerGone)? + } + async fn send(&self, req: Request) -> Result<(), DbError> { self.inbox.send(req).await.map_err(|_| DbError::WorkerGone) } @@ -1044,9 +1106,108 @@ fn handle_request(conn: &Connection, persistence: Option<&Persistence>, req: Req &project_path, )); } + Request::ReadRelationships { table, reply } => { + let result = do_read_relationships(conn, &table); + let _ = reply.send(result); + } + Request::FindRowsMatching { + table, + column, + value, + limit, + reply, + } => { + let result = do_find_rows_matching(conn, &table, &column, &value, limit); + let _ = reply.send(result); + } } } +/// Read both directions of FK relationships for `table`. Used +/// by `Request::ReadRelationships` (ADR-0019 §6 enrichment). +fn do_read_relationships(conn: &Connection, table: &str) -> RelationshipsReply { + let outbound = read_relationships_outbound(conn, table)?; + let inbound = read_relationships_inbound(conn, table)?; + Ok((outbound, inbound)) +} + +/// `SELECT * FROM WHERE = LIMIT `. +/// Used by the runtime to pinpoint rows after a UNIQUE / FK +/// violation (ADR-0019 §6, ADR-0017 §7). Returns +/// `DbError::Sqlite` on bind failure, missing column, etc. — +/// callers treat any error as "no diagnostic table available" +/// and fall back to the headline-only wording. +fn do_find_rows_matching( + conn: &Connection, + table: &str, + column: &str, + value: &Value, + limit: usize, +) -> Result { + let schema = read_schema(conn, table)?; + let col_info = schema + .columns + .iter() + .find(|c| c.name == column) + .ok_or_else(|| DbError::Sqlite { + message: format!("no such column: {table}.{column}"), + kind: SqliteErrorKind::NoSuchColumn, + })?; + let ty = col_info.user_type.ok_or_else(|| { + DbError::Unsupported(format!( + "column `{column}` has no user-type metadata; cannot pinpoint" + )) + })?; + let bound = value + .bind_for_column(column, ty) + .map_err(|e| DbError::InvalidValue(e.to_string()))?; + + let column_names: Vec = schema.columns.iter().map(|c| c.name.clone()).collect(); + let column_types: Vec> = + schema.columns.iter().map(|c| c.user_type).collect(); + let cols_csv = column_names + .iter() + .map(|c| quote_ident(c)) + .collect::>() + .join(", "); + let sql = format!( + "SELECT {cols} FROM {tbl} WHERE {col} = ?1 LIMIT {n};", + cols = cols_csv, + tbl = quote_ident(table), + col = quote_ident(column), + n = limit, + ); + + let bound_value = bound_to_sqlite_value(&bound); + let mut stmt = conn.prepare(&sql).map_err(DbError::from_rusqlite)?; + let rows_iter = stmt + .query_map(rusqlite::params![bound_value], |row| { + let mut cells: Vec = + Vec::with_capacity(column_names.len()); + for i in 0..column_names.len() { + cells.push(row.get(i)?); + } + Ok(cells) + }) + .map_err(DbError::from_rusqlite)?; + let mut rows: Vec>> = Vec::new(); + for r in rows_iter { + let cells = r.map_err(DbError::from_rusqlite)?; + let formatted: Vec> = cells + .into_iter() + .zip(column_types.iter()) + .map(|(v, ty)| format_cell(v, *ty)) + .collect(); + rows.push(formatted); + } + Ok(DataResult { + table_name: table.to_string(), + columns: column_names, + column_types, + rows, + }) +} + /// Set of changes a mutation made, used by the post-mutation /// persistence phase to know which text targets need refreshing /// (ADR-0015 §6). @@ -1993,8 +2154,8 @@ fn do_change_column_type( .map_err(|e| { let ctx = crate::friendly::TranslateContext { operation: Some(crate::friendly::Operation::ChangeColumnType), - table: Some(table), - column: Some(column), + table: Some(table.to_string()), + column: Some(column.to_string()), src_type: Some(src_ty), target_type: Some(ty), ..crate::friendly::TranslateContext::default() @@ -3513,8 +3674,9 @@ fn do_insert( if user_cols.len() != user_values.len() { return Err(DbError::InvalidValue(format!( - "expected {} value(s), got {}", + "expected {} value(s) for ({}), got {}", user_cols.len(), + user_cols.join(", "), user_values.len() ))); } diff --git a/src/event.rs b/src/event.rs index 5cbb79a..03fafce 100644 --- a/src/event.rs +++ b/src/event.rs @@ -57,12 +57,15 @@ pub enum AppEvent { result: AddColumnResult, }, /// A DSL command failed. `error` is the structured - /// payload — App applies its current verbosity setting - /// (`messages_verbosity`) when rendering through - /// `friendly::translate_error` (ADR-0019 §5). + /// payload, `facts` is the runtime-built schema-resolved + /// enrichment (parent tables, attempted values, + /// pinpointed offending rows). App applies its current + /// verbosity setting (`messages_verbosity`) when rendering + /// through `friendly::translate_error` (ADR-0019 §5, §6). DslFailed { command: Command, error: DbError, + facts: crate::friendly::FailureContext, }, /// Refreshed list of tables in the database. TablesRefreshed(Vec), diff --git a/src/friendly/mod.rs b/src/friendly/mod.rs index 738a074..06a6f0f 100644 --- a/src/friendly/mod.rs +++ b/src/friendly/mod.rs @@ -35,7 +35,7 @@ pub mod translate; pub use error::{DiagnosticTable, FriendlyError}; pub use format::{catalog, Catalog}; -pub use translate::{Operation, TranslateContext, Verbosity}; +pub use translate::{FailureContext, Operation, TranslateContext, Verbosity}; // `translate::translate` and `format::translate` are different // callables — the former is the structured DbError → FriendlyError diff --git a/src/friendly/translate.rs b/src/friendly/translate.rs index f4ef2dc..7cd5be6 100644 --- a/src/friendly/translate.rs +++ b/src/friendly/translate.rs @@ -31,7 +31,7 @@ use crate::db::{DbError, SqliteErrorKind}; use crate::dsl::Type; -use crate::friendly::error::FriendlyError; +use crate::friendly::error::{DiagnosticTable, FriendlyError}; use crate::t; /// Verbosity of the rendered error. @@ -100,29 +100,76 @@ impl Operation { } } +/// Schema-resolved facts about a failure (ADR-0019 §6). +/// +/// Built by the runtime (where the `Database` handle is +/// available) and passed to the App via +/// [`crate::event::AppEvent::DslFailed`]. The App combines a +/// `FailureContext` with its current verbosity and the +/// operation derived from the originating `Command` to build a +/// [`TranslateContext`]. +/// +/// Every field is optional — the runtime fills what it can +/// resolve and leaves the rest `None`. The translator falls +/// back to its `{name}`-form placeholders where data is +/// missing. +#[derive(Debug, Clone, Default)] +pub struct FailureContext { + /// Operation table from the command (may differ from + /// `parent_table` for FK violations). + pub table: Option, + /// Column name resolved from engine error (`UNIQUE + /// constraint failed: T.col` etc.) or via FK relationship + /// lookup. + pub column: Option, + /// User's attempted value for the offending column. + pub value: Option, + /// For child-side FK violations: the parent table the FK + /// references. + pub parent_table: Option, + /// For child-side FK violations: the parent column the + /// FK references. + pub parent_column: Option, + /// For parent-side FK violations: a child table that + /// references this row. + pub child_table: Option, + /// Pinpointed offending row(s) per ADR-0019 §6 / ADR-0017 + /// §7. Rendered through the bordered diagnostic-table + /// renderer when present. + pub diagnostic_table: Option, +} + /// Context the translator uses to pick catalog keys and fill /// placeholders. Every field is optional — the translator falls -/// back to abstract wording where context is missing. +/// back to its `{name}`-form placeholders where context is +/// missing. #[derive(Debug, Clone, Default)] -pub struct TranslateContext<'a> { +pub struct TranslateContext { pub operation: Option, - pub table: Option<&'a str>, - pub column: Option<&'a str>, + pub table: Option, + pub column: Option, /// For parent-side FK violations: the child table that /// references this row. Surfaced as `{child_table}` in /// `error.foreign_key.parent_side.*`. - pub child_table: Option<&'a str>, + pub child_table: Option, + /// For child-side FK violations: the parent table the FK + /// references. Surfaced as `{parent_table}` in + /// `error.foreign_key.child_side.*`. + pub parent_table: Option, + /// For child-side FK violations: the parent column the FK + /// references. Surfaced as `{parent_column}`. + pub parent_column: Option, pub src_type: Option, pub target_type: Option, - /// User-attempted value for INSERT/UPDATE; surfaced as the - /// `{value}` placeholder in UNIQUE / FK / type-mismatch - /// wording. Best-effort: callsites populate when they have - /// it; translator falls back to `` otherwise. + /// User-attempted value for INSERT/UPDATE. pub value: Option, + /// Pinpointed offending row(s); rendered onto the + /// `FriendlyError::diagnostic_table` field when present. + pub diagnostic_table: Option, pub verbosity: Verbosity, } -impl<'a> TranslateContext<'a> { +impl TranslateContext { /// Convenience constructor for the common "I just have an /// operation" case. #[must_use] @@ -132,13 +179,36 @@ impl<'a> TranslateContext<'a> { ..Self::default() } } + + /// Combine schema-resolved facts with operation and + /// verbosity to build the full translator input. + #[must_use] + pub fn from_facts( + operation: Operation, + verbosity: Verbosity, + facts: FailureContext, + ) -> Self { + Self { + operation: Some(operation), + table: facts.table, + column: facts.column, + child_table: facts.child_table, + parent_table: facts.parent_table, + parent_column: facts.parent_column, + src_type: None, + target_type: None, + value: facts.value, + diagnostic_table: facts.diagnostic_table, + verbosity, + } + } } /// Classify `error` and produce a structured [`FriendlyError`]. /// See module docs for the classification flow. #[must_use] -pub fn translate(error: &DbError, ctx: &TranslateContext<'_>) -> FriendlyError { - match error { +pub fn translate(error: &DbError, ctx: &TranslateContext) -> FriendlyError { + let mut fe = match error { DbError::Sqlite { message, kind } => translate_sqlite(message, *kind, ctx), // Unsupported / InvalidValue carry text that is already // engine-neutral and friendly (constructed by our own @@ -154,13 +224,20 @@ pub fn translate(error: &DbError, ctx: &TranslateContext<'_>) -> FriendlyError { DbError::WorkerGone => passthrough( "the database worker is no longer available — the application must restart", ), + }; + // Attach the row pinpoint when the runtime resolved one. + // The translator never builds the table itself — it only + // forwards what enrichment supplied. + if fe.diagnostic_table.is_none() { + fe.diagnostic_table = ctx.diagnostic_table.clone(); } + fe } fn translate_sqlite( message: &str, kind: SqliteErrorKind, - ctx: &TranslateContext<'_>, + ctx: &TranslateContext, ) -> FriendlyError { // `change column ... --dont-convert` lets the engine // accept or refuse each cell. Whatever the engine returns @@ -184,7 +261,7 @@ fn translate_sqlite( } } -fn translate_type_mismatch_change_column(ctx: &TranslateContext<'_>) -> FriendlyError { +fn translate_type_mismatch_change_column(ctx: &TranslateContext) -> FriendlyError { let table = ctx_table(ctx); let column = ctx_column(ctx); let src_type = ctx @@ -211,7 +288,7 @@ fn translate_type_mismatch_change_column(ctx: &TranslateContext<'_>) -> Friendly ) } -fn translate_constraint(message: &str, ctx: &TranslateContext<'_>) -> FriendlyError { +fn translate_constraint(message: &str, ctx: &TranslateContext) -> FriendlyError { let lower = message.to_ascii_lowercase(); if lower.contains("unique constraint failed") { translate_unique(message, ctx) @@ -228,7 +305,7 @@ fn translate_constraint(message: &str, ctx: &TranslateContext<'_>) -> FriendlyEr // ---- UNIQUE ----------------------------------------------------- -fn translate_unique(message: &str, ctx: &TranslateContext<'_>) -> FriendlyError { +fn translate_unique(message: &str, ctx: &TranslateContext) -> FriendlyError { let (table, column) = parse_qualified_target(message) .unwrap_or_else(|| (ctx_table(ctx), ctx_column(ctx))); let value = ctx_value(ctx); @@ -274,27 +351,41 @@ fn translate_unique(message: &str, ctx: &TranslateContext<'_>) -> FriendlyError // ---- FOREIGN KEY ----------------------------------------------- -fn translate_foreign_key(ctx: &TranslateContext<'_>) -> FriendlyError { +fn translate_foreign_key(ctx: &TranslateContext) -> FriendlyError { // The engine's "FOREIGN KEY constraint failed" carries no - // detail. Disambiguation is operation-driven: child-side - // happens on INSERT/UPDATE (the row being written points at - // a missing parent); parent-side happens on DELETE/UPDATE - // (the row being deleted is referenced by a child). + // detail. Disambiguation is enrichment-driven first + // (`parent_table` populated → child-side; `child_table` + // populated → parent-side), with operation as the + // tiebreaker when enrichment didn't run. // - // Without context we default to child-side INSERT, which - // is the more common case and matches the wording of the - // pre-H1 enrich_fk_message helper. + // - Insert always points "outward" → child-side. + // - Delete always points "inward" → parent-side. + // - Update can be either; we let the enrichment payload + // choose, defaulting to child-side (the more pedagogically + // common case for a learner). + if ctx.parent_table.is_some() { + return match ctx.operation { + Some(Operation::Update) => fk_child_side_update(ctx), + _ => fk_child_side_insert(ctx), + }; + } + if ctx.child_table.is_some() { + return match ctx.operation { + Some(Operation::Update) => fk_parent_side_update(ctx), + _ => fk_parent_side_delete(ctx), + }; + } match ctx.operation { Some(Operation::Delete) => fk_parent_side_delete(ctx), - Some(Operation::Update) => fk_parent_side_update(ctx), + Some(Operation::Update) => fk_child_side_update(ctx), Some(Operation::Insert) => fk_child_side_insert(ctx), _ => fk_child_side_insert(ctx), } } -fn fk_child_side_insert(ctx: &TranslateContext<'_>) -> FriendlyError { - let parent_table = ctx_table(ctx); - let parent_column = ctx_column(ctx); +fn fk_child_side_insert(ctx: &TranslateContext) -> FriendlyError { + let parent_table = ctx_parent_table(ctx); + let parent_column = ctx_parent_column(ctx); let value = ctx_value(ctx); fe( t!( @@ -314,11 +405,31 @@ fn fk_child_side_insert(ctx: &TranslateContext<'_>) -> FriendlyError { ) } -fn fk_parent_side_delete(ctx: &TranslateContext<'_>) -> FriendlyError { +fn fk_child_side_update(ctx: &TranslateContext) -> FriendlyError { + let parent_table = ctx_parent_table(ctx); + let parent_column = ctx_parent_column(ctx); + let value = ctx_value(ctx); + fe( + t!( + "error.foreign_key.child_side.update.headline", + parent_table = parent_table, + parent_column = parent_column, + value = value + ), + verbose_hint( + ctx, + t!( + "error.foreign_key.child_side.update.hint", + parent_table = parent_table, + parent_column = parent_column + ), + ), + ) +} + +fn fk_parent_side_delete(ctx: &TranslateContext) -> FriendlyError { let table = ctx_table(ctx); - let child_table = ctx - .child_table - .map_or_else(|| "another table".to_string(), str::to_string); + let child_table = ctx_child_table(ctx); fe( t!( "error.foreign_key.parent_side.delete.headline", @@ -329,11 +440,9 @@ fn fk_parent_side_delete(ctx: &TranslateContext<'_>) -> FriendlyError { ) } -fn fk_parent_side_update(ctx: &TranslateContext<'_>) -> FriendlyError { +fn fk_parent_side_update(ctx: &TranslateContext) -> FriendlyError { let table = ctx_table(ctx); - let child_table = ctx - .child_table - .map_or_else(|| "another table".to_string(), str::to_string); + let child_table = ctx_child_table(ctx); fe( t!( "error.foreign_key.parent_side.update.headline", @@ -346,7 +455,7 @@ fn fk_parent_side_update(ctx: &TranslateContext<'_>) -> FriendlyError { // ---- NOT NULL -------------------------------------------------- -fn translate_not_null(message: &str, ctx: &TranslateContext<'_>) -> FriendlyError { +fn translate_not_null(message: &str, ctx: &TranslateContext) -> FriendlyError { let (table, column) = parse_qualified_target(message) .unwrap_or_else(|| (ctx_table(ctx), ctx_column(ctx))); match ctx.operation { @@ -371,7 +480,7 @@ fn translate_not_null(message: &str, ctx: &TranslateContext<'_>) -> FriendlyErro // ---- CHECK ----------------------------------------------------- -fn translate_check(_message: &str, ctx: &TranslateContext<'_>) -> FriendlyError { +fn translate_check(_message: &str, ctx: &TranslateContext) -> FriendlyError { // The engine reports CHECK constraint failures by constraint // name, not by column. We don't have user-named CHECK // constraints today, so the message is rarely informative. @@ -400,13 +509,13 @@ fn translate_check(_message: &str, ctx: &TranslateContext<'_>) -> FriendlyError // ---- not_found / already_exists -------------------------------- -fn translate_not_found_table(message: &str, ctx: &TranslateContext<'_>) -> FriendlyError { +fn translate_not_found_table(message: &str, ctx: &TranslateContext) -> FriendlyError { let name = parse_after_colon(message) .map_or_else(|| ctx_table(ctx), str::to_string); headline_only(t!("error.not_found.table.headline", name = name)) } -fn translate_not_found_column(message: &str, ctx: &TranslateContext<'_>) -> FriendlyError { +fn translate_not_found_column(message: &str, ctx: &TranslateContext) -> FriendlyError { let name = parse_after_colon(message).unwrap_or(""); if let Some((table, column)) = name.split_once('.') { headline_only(t!( @@ -428,7 +537,7 @@ fn translate_not_found_column(message: &str, ctx: &TranslateContext<'_>) -> Frie } } -fn translate_already_exists(message: &str, ctx: &TranslateContext<'_>) -> FriendlyError { +fn translate_already_exists(message: &str, ctx: &TranslateContext) -> FriendlyError { // Three shapes feed in: // - Engine: "table T already exists" // - Our own: "column `T.col` already exists; pick a different name." @@ -469,7 +578,7 @@ fn translate_already_exists(message: &str, ctx: &TranslateContext<'_>) -> Friend // ---- Generic catch-all ----------------------------------------- -fn translate_generic(_message: &str, ctx: &TranslateContext<'_>) -> FriendlyError { +fn translate_generic(_message: &str, ctx: &TranslateContext) -> FriendlyError { // Engine message is intentionally NOT surfaced — ADR-0002 // posture. The catalog provides the abstract wording. let operation = ctx @@ -508,7 +617,7 @@ const fn fe(headline: String, hint: Option) -> FriendlyError { } } -fn verbose_hint(ctx: &TranslateContext<'_>, hint: String) -> Option { +fn verbose_hint(ctx: &TranslateContext, hint: String) -> Option { if ctx.verbosity == Verbosity::Verbose { Some(hint) } else { @@ -520,23 +629,34 @@ fn verbose_hint(ctx: &TranslateContext<'_>, hint: String) -> Option { // the catalog's `{name}` form so unfilled positions read as // "this placeholder was not supplied" — same shape the // translator's source uses, easier to grep, and visually -// consistent with the catalog templates. Filling these -// properly across the board needs schema-aware enrichment in -// the runtime; that work is bundled with the row-pinpoint -// re-query (ADR-0019 §6) since both need the Database handle. +// consistent with the catalog templates. With runtime-side +// enrichment (ADR-0019 §6) populating `FailureContext`, +// these fallbacks rarely render in practice. -fn ctx_table(ctx: &TranslateContext<'_>) -> String { - ctx.table.map_or_else(|| "{table}".to_string(), str::to_string) +fn ctx_table(ctx: &TranslateContext) -> String { + ctx.table.clone().unwrap_or_else(|| "{table}".to_string()) } -fn ctx_column(ctx: &TranslateContext<'_>) -> String { - ctx.column.map_or_else(|| "{column}".to_string(), str::to_string) +fn ctx_column(ctx: &TranslateContext) -> String { + ctx.column.clone().unwrap_or_else(|| "{column}".to_string()) } -fn ctx_value(ctx: &TranslateContext<'_>) -> String { +fn ctx_value(ctx: &TranslateContext) -> String { ctx.value.clone().unwrap_or_else(|| "{value}".to_string()) } +fn ctx_parent_table(ctx: &TranslateContext) -> String { + ctx.parent_table.clone().unwrap_or_else(|| "{parent_table}".to_string()) +} + +fn ctx_parent_column(ctx: &TranslateContext) -> String { + ctx.parent_column.clone().unwrap_or_else(|| "{parent_column}".to_string()) +} + +fn ctx_child_table(ctx: &TranslateContext) -> String { + ctx.child_table.clone().unwrap_or_else(|| "{child_table}".to_string()) +} + /// Extract `T.col` from a message like /// `"UNIQUE constraint failed: T.col"`. Returns `(T, col)` or /// `None` if the message doesn't have the expected shape. @@ -587,7 +707,7 @@ fn parse_after_word<'a>(message: &'a str, keyword: &str) -> Option<&'a str> { mod tests { use super::*; - fn ctx_with(op: Operation) -> TranslateContext<'static> { + fn ctx_with(op: Operation) -> TranslateContext { TranslateContext { operation: Some(op), ..TranslateContext::default() @@ -665,8 +785,8 @@ mod tests { SqliteErrorKind::UniqueViolation, ); let mut ctx = ctx_with(Operation::Insert); - ctx.table = Some("Customers"); - ctx.column = Some("id"); + ctx.parent_table = Some("Customers".to_string()); + ctx.parent_column = Some("id".to_string()); ctx.value = Some("99".to_string()); let f = translate(&err, &ctx); assert!( @@ -674,6 +794,7 @@ mod tests { "expected child-side phrasing: {}", f.headline ); + assert!(f.headline.contains("Customers")); assert!(f.headline.contains("`99`")); } @@ -684,8 +805,8 @@ mod tests { SqliteErrorKind::UniqueViolation, ); let mut ctx = ctx_with(Operation::Delete); - ctx.table = Some("Customers"); - ctx.child_table = Some("Orders"); + ctx.table = Some("Customers".to_string()); + ctx.child_table = Some("Orders".to_string()); let f = translate(&err, &ctx); // Anchor phrase: "referenced by". assert!( @@ -720,8 +841,8 @@ mod tests { SqliteErrorKind::UniqueViolation, ); let mut ctx = ctx_with(Operation::Insert); - ctx.table = Some("People"); - ctx.column = Some("age"); + ctx.table = Some("People".to_string()); + ctx.column = Some("age".to_string()); let f = translate(&err, &ctx); assert!(f.headline.contains("check constraint refused")); assert!(f.headline.contains("People")); diff --git a/src/runtime.rs b/src/runtime.rs index 0e9fe84..2bdcfa3 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -972,10 +972,17 @@ fn spawn_dsl_dispatch( path, message, }, - Err(error) => AppEvent::DslFailed { - command: command.clone(), - error, - }, + Err(error) => { + // Schema-resolved enrichment per ADR-0019 §6. + // The runtime owns DB access; the App stays + // presentation-only. + let facts = enrich_dsl_failure(&database, &command, &error).await; + AppEvent::DslFailed { + command: command.clone(), + error, + facts, + } + } }; if event_tx.send(event).await.is_err() { return; @@ -993,6 +1000,286 @@ fn spawn_dsl_dispatch( }); } +/// Build schema-resolved enrichment for a DSL failure (ADR-0019 §6). +/// +/// Best-effort: every lookup is independently fallible and a +/// missing piece just leaves the corresponding +/// `FailureContext` field `None`. The translator falls back to +/// catalog `{name}` placeholders for unfilled fields. +/// +/// What we resolve, by classification: +/// +/// - **UNIQUE / NOT NULL violation** (engine reports `T.col`): +/// - `table`, `column` from the engine message. +/// - `value` from the originating Command (explicit columns +/// or single-value short form, with schema lookup as a +/// last resort for natural-order multi-value INSERT). +/// - For UNIQUE only: `diagnostic_table` from a pinpoint +/// `SELECT * FROM T WHERE col = value LIMIT N` showing +/// the existing row that conflicts. +/// - **FK INSERT/UPDATE** (child-side): outbound relationship +/// lookup picks the FK column the user set; resolves +/// `parent_table`, `parent_column`, and the attempted +/// `value`. +/// - **FK DELETE/UPDATE** (parent-side): inbound relationship +/// lookup picks a `child_table` that references this row. +/// - Anything else: `FailureContext::default()`. +pub async fn enrich_dsl_failure( + database: &Database, + command: &Command, + error: &DbError, +) -> crate::friendly::FailureContext { + let DbError::Sqlite { message, .. } = error else { + return crate::friendly::FailureContext::default(); + }; + let lower = message.to_ascii_lowercase(); + if lower.contains("unique constraint failed") { + enrich_unique_violation(database, command, message).await + } else if lower.contains("not null constraint failed") { + enrich_not_null_violation(command, message) + } else if lower.contains("foreign key constraint failed") { + enrich_fk_violation(database, command).await + } else { + crate::friendly::FailureContext::default() + } +} + +async fn enrich_unique_violation( + database: &Database, + command: &Command, + message: &str, +) -> crate::friendly::FailureContext { + let mut facts = crate::friendly::FailureContext::default(); + let Some((table, column)) = parse_qualified_target(message) else { + return facts; + }; + facts.table = Some(table.clone()); + facts.column = Some(column.clone()); + + // Resolve the user's attempted value. + let raw_value = user_value_for_column_with_schema(database, command, &table, &column).await; + facts.value = raw_value.as_ref().map(ToString::to_string); + + // Pinpoint the existing conflicting row, capped per + // ADR-0017 §7's `DIAGNOSTIC_ROW_CAP` (we use a tighter cap + // here — a single conflicting row is the typical case for + // UNIQUE since the constraint enforces it). + if let Some(value) = raw_value + && let Ok(data) = database + .find_rows_matching(table.clone(), column.clone(), value, 5) + .await + && !data.rows.is_empty() + { + facts.diagnostic_table = Some(diagnostic_from_data_result(&data)); + } + facts +} + +fn enrich_not_null_violation( + command: &Command, + message: &str, +) -> crate::friendly::FailureContext { + let mut facts = crate::friendly::FailureContext::default(); + let Some((table, column)) = parse_qualified_target(message) else { + return facts; + }; + facts.table = Some(table); + facts.column = Some(column); + // The "attempted value" for NOT NULL is by definition null — + // surfacing it doesn't add information. Skip the value + // resolution; the catalog headline reads "X cannot be null" + // and stands on its own. + let _ = command; + facts +} + +async fn enrich_fk_violation( + database: &Database, + command: &Command, +) -> crate::friendly::FailureContext { + let mut facts = crate::friendly::FailureContext::default(); + match command { + Command::Insert { table, .. } | Command::Update { table, .. } => { + // Child-side: outbound FK lookup. Find the FK + // column the user is setting / updating. Use the + // schema-aware lookup so natural-order multi-value + // INSERT (which `user_value_for_column` alone can't + // resolve) gets handled too. + let Ok((outbound, _)) = + database.read_relationships(table.clone()).await + else { + return facts; + }; + facts.table = Some(table.clone()); + for rel in outbound { + let value = user_value_for_column_with_schema( + database, + command, + table, + &rel.local_column, + ) + .await; + if let Some(v) = value { + facts.column = Some(rel.local_column); + facts.parent_table = Some(rel.other_table); + facts.parent_column = Some(rel.other_column); + facts.value = Some(v.to_string()); + break; + } + } + // For UPDATE, if no outbound match was found we may + // be in the parent-side case (updating a column + // children reference). Check inbound as a fallback. + if facts.parent_table.is_none() + && matches!(command, Command::Update { .. }) + && let Ok((_, inbound)) = + database.read_relationships(table.clone()).await + && let Some(rel) = inbound.first() + { + facts.child_table = Some(rel.other_table.clone()); + } + } + Command::Delete { table, .. } => { + // Parent-side: inbound FK lookup. Surface a child + // table that still references the row(s) being + // deleted. + let Ok((_, inbound)) = + database.read_relationships(table.clone()).await + else { + return facts; + }; + facts.table = Some(table.clone()); + if let Some(rel) = inbound.first() { + facts.child_table = Some(rel.other_table.clone()); + } + } + _ => {} + } + facts +} + +/// Find the user's attempted value for `column` directly from +/// the originating Command. Handles INSERT (explicit columns, +/// single-value short form) and UPDATE (assignments). Returns +/// `None` for natural-order multi-value INSERT — that case +/// needs a schema lookup, see +/// [`user_value_for_column_with_schema`]. +fn user_value_for_column(command: &Command, column: &str) -> Option { + match command { + Command::Insert { + columns, values, .. + } => { + if let Some(cols) = columns { + let idx = cols.iter().position(|c| c == column)?; + values.get(idx).cloned() + } else if values.len() == 1 { + values.first().cloned() + } else { + None + } + } + Command::Update { assignments, .. } => assignments + .iter() + .find(|(c, _)| c == column) + .map(|(_, v)| v.clone()), + _ => None, + } +} + +/// Same as [`user_value_for_column`] but handles natural-order +/// multi-value INSERT by reading the schema to learn which +/// position belongs to which column. Mirrors `do_insert`'s +/// position-mapping rule (auto-generated columns — serial, +/// shortid — are skipped, since the user doesn't supply +/// values for them in the natural-order short form). +async fn user_value_for_column_with_schema( + database: &Database, + command: &Command, + table: &str, + column: &str, +) -> Option { + if let Some(v) = user_value_for_column(command, column) { + return Some(v); + } + if let Command::Insert { + columns: None, + values, + .. + } = command + { + let desc = database + .describe_table(table.to_string(), None) + .await + .ok()?; + // Build the natural-order column list the same way + // `do_insert` does: filter out serial / shortid columns + // because the engine auto-fills them and the user's + // positional values map onto the remainder. + let natural_cols: Vec<&str> = desc + .columns + .iter() + .filter(|c| { + !matches!( + c.user_type, + Some(crate::dsl::Type::Serial) + | Some(crate::dsl::Type::ShortId) + ) + }) + .map(|c| c.name.as_str()) + .collect(); + let idx = natural_cols.iter().position(|c| *c == column)?; + return values.get(idx).cloned(); + } + None +} + +/// Render a `DataResult` as a `DiagnosticTable` for the +/// friendly-error layer's bordered renderer (ADR-0019 §7, +/// reusing ADR-0017 §7's renderer). +fn diagnostic_from_data_result( + data: &DataResult, +) -> crate::friendly::DiagnosticTable { + use crate::output_render::{numeric_alignment_for, Alignment}; + let alignments: Vec = data + .column_types + .iter() + .map(|t| { + t.map_or(Alignment::Left, numeric_alignment_for) + }) + .collect(); + let rows: Vec> = data + .rows + .iter() + .map(|r| { + r.iter() + .map(|c| c.clone().unwrap_or_else(|| "NULL".to_string())) + .collect() + }) + .collect(); + crate::friendly::DiagnosticTable { + headers: data.columns.clone(), + rows, + alignments, + } +} + +/// Extract `(table, column)` from a qualified target like +/// `"UNIQUE constraint failed: T.col"`. Mirrors the helper in +/// `friendly::translate` (the translator does its own parse as +/// a fallback when enrichment didn't run). +fn parse_qualified_target(message: &str) -> Option<(String, String)> { + let after = message.split_once(':').map(|(_, r)| r.trim())?; + let first = after.split(',').next()?.trim(); + let mut parts = first.splitn(2, '.'); + let table = parts.next()?.trim(); + let column = parts.next()?.trim(); + if table.is_empty() || column.is_empty() { + None + } else { + Some((table.to_string(), column.to_string())) + } +} + enum CommandOutcome { Schema(Option), Query(DataResult), diff --git a/tests/friendly_enrichment.rs b/tests/friendly_enrichment.rs new file mode 100644 index 0000000..7d7366b --- /dev/null +++ b/tests/friendly_enrichment.rs @@ -0,0 +1,506 @@ +//! Integration tests for `runtime::enrich_dsl_failure` +//! (ADR-0019 §6). +//! +//! Each test: +//! 1. Bootstraps a real `Database` (in-memory). +//! 2. Constructs the schema/data needed to trigger one +//! class of engine error. +//! 3. Provokes the failure through the public Database API, +//! capturing the resulting `DbError`. +//! 4. Calls `enrich_dsl_failure` and asserts the +//! `FailureContext` carries the schema-resolved facts a +//! learner would expect to see in the rendered error. +//! +//! Pinpoint diagnostic-table presence is verified for the +//! UNIQUE INSERT case (the most pedagogically valuable +//! pinpoint today). + +use tokio::runtime::Runtime; + +use rdbms_playground::db::{Database, DbError, SqliteErrorKind}; +use rdbms_playground::dsl::{ + action::ReferentialAction, ColumnSpec, Command, RowFilter, Type, Value, +}; +use rdbms_playground::runtime::enrich_dsl_failure; + +fn rt() -> Runtime { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("tokio rt") +} + +fn db() -> Database { + Database::open(":memory:").expect("open in-memory db") +} + +// ---- UNIQUE ----------------------------------------------------- + +#[test] +fn enrich_unique_insert_resolves_table_column_value_and_pinpoint() { + let db = db(); + rt().block_on(async { + // Create a table with a serial PK; insert a row; insert + // again with the same PK value to trigger UNIQUE. + db.create_table( + "Customers".to_string(), + vec![ + ColumnSpec { name: "id".to_string(), ty: Type::Int }, + ColumnSpec { name: "name".to_string(), ty: Type::Text }, + ], + vec!["id".to_string()], + None, + ) + .await + .unwrap(); + db.insert( + "Customers".to_string(), + None, + vec![Value::Number("5".to_string()), Value::Text("Alice".to_string())], + None, + ) + .await + .unwrap(); + + // Second insert with the same PK — UNIQUE violation. + let cmd = Command::Insert { + table: "Customers".to_string(), + columns: Some(vec!["id".to_string(), "name".to_string()]), + values: vec![ + Value::Number("5".to_string()), + Value::Text("Bob".to_string()), + ], + }; + let err = db + .insert( + "Customers".to_string(), + Some(vec!["id".to_string(), "name".to_string()]), + vec![ + Value::Number("5".to_string()), + Value::Text("Bob".to_string()), + ], + None, + ) + .await + .unwrap_err(); + assert!(matches!( + err, + DbError::Sqlite { kind: SqliteErrorKind::UniqueViolation, .. } + )); + + let facts = enrich_dsl_failure(&db, &cmd, &err).await; + assert_eq!(facts.table.as_deref(), Some("Customers")); + assert_eq!(facts.column.as_deref(), Some("id")); + assert_eq!(facts.value.as_deref(), Some("5")); + // Pinpoint: existing row with id=5 should be present. + let table = facts.diagnostic_table.expect("UNIQUE pinpoint expected"); + assert_eq!(table.headers, vec!["id".to_string(), "name".to_string()]); + assert_eq!(table.rows.len(), 1); + assert_eq!(table.rows[0][0], "5"); + assert_eq!(table.rows[0][1], "Alice"); + }); +} + +#[test] +fn enrich_unique_insert_natural_order_short_form_resolves_value_via_schema() { + // `insert into T (1)` — natural-order short form, the + // helper falls back to schema-driven lookup. + let db = db(); + rt().block_on(async { + db.create_table( + "thing".to_string(), + vec![ColumnSpec { name: "id".to_string(), ty: Type::Int }], + vec!["id".to_string()], + None, + ) + .await + .unwrap(); + db.insert( + "thing".to_string(), + None, + vec![Value::Number("1".to_string())], + None, + ) + .await + .unwrap(); + + let cmd = Command::Insert { + table: "thing".to_string(), + columns: None, + values: vec![Value::Number("1".to_string())], + }; + let err = db + .insert( + "thing".to_string(), + None, + vec![Value::Number("1".to_string())], + None, + ) + .await + .unwrap_err(); + + let facts = enrich_dsl_failure(&db, &cmd, &err).await; + assert_eq!(facts.value.as_deref(), Some("1")); + assert!(facts.diagnostic_table.is_some()); + }); +} + +#[test] +fn enrich_unique_update_resolves_value_from_assignments() { + let db = db(); + rt().block_on(async { + db.create_table( + "Customers".to_string(), + vec![ + ColumnSpec { name: "id".to_string(), ty: Type::Int }, + ColumnSpec { name: "name".to_string(), ty: Type::Text }, + ], + vec!["id".to_string()], + None, + ) + .await + .unwrap(); + db.insert( + "Customers".to_string(), + None, + vec![Value::Number("1".to_string()), Value::Text("Alice".to_string())], + None, + ) + .await + .unwrap(); + db.insert( + "Customers".to_string(), + None, + vec![Value::Number("2".to_string()), Value::Text("Bob".to_string())], + None, + ) + .await + .unwrap(); + + // Try to update Bob's id to 1 — collides with Alice. + let cmd = Command::Update { + table: "Customers".to_string(), + assignments: vec![("id".to_string(), Value::Number("1".to_string()))], + filter: RowFilter::Where { + column: "name".to_string(), + value: Value::Text("Bob".to_string()), + }, + }; + let err = db + .update( + "Customers".to_string(), + vec![("id".to_string(), Value::Number("1".to_string()))], + RowFilter::Where { + column: "name".to_string(), + value: Value::Text("Bob".to_string()), + }, + None, + ) + .await + .unwrap_err(); + + let facts = enrich_dsl_failure(&db, &cmd, &err).await; + assert_eq!(facts.column.as_deref(), Some("id")); + assert_eq!(facts.value.as_deref(), Some("1")); + }); +} + +// ---- NOT NULL --------------------------------------------------- + +#[test] +fn enrich_not_null_resolves_table_and_column() { + let db = db(); + rt().block_on(async { + // Create a table with a NOT NULL column. The current + // schema_to_ddl emits NOT NULL on PK columns; make + // a non-PK column NOT NULL via a multi-column PK + // setup, then the second column is NOT NULL because + // it's part of the PK. + // (We're testing the enrichment, not the constraint + // emission — even a PK NOT NULL works.) + db.create_table( + "T".to_string(), + vec![ + ColumnSpec { name: "a".to_string(), ty: Type::Int }, + ColumnSpec { name: "b".to_string(), ty: Type::Text }, + ], + vec!["a".to_string(), "b".to_string()], + None, + ) + .await + .unwrap(); + + // Try to insert with NULL for the second PK column. + let cmd = Command::Insert { + table: "T".to_string(), + columns: Some(vec!["a".to_string(), "b".to_string()]), + values: vec![Value::Number("1".to_string()), Value::Null], + }; + let err = db + .insert( + "T".to_string(), + Some(vec!["a".to_string(), "b".to_string()]), + vec![Value::Number("1".to_string()), Value::Null], + None, + ) + .await + .unwrap_err(); + + let facts = enrich_dsl_failure(&db, &cmd, &err).await; + assert_eq!(facts.table.as_deref(), Some("T")); + assert_eq!(facts.column.as_deref(), Some("b")); + // Per design: no value field for NOT NULL (the value is null). + assert!(facts.value.is_none()); + // No pinpoint for NOT NULL. + assert!(facts.diagnostic_table.is_none()); + }); +} + +// ---- FOREIGN KEY (child-side, INSERT) --------------------------- + +#[test] +fn enrich_fk_insert_resolves_parent_table_column_and_value() { + let db = db(); + rt().block_on(async { + db.create_table( + "Customers".to_string(), + vec![ColumnSpec { name: "id".to_string(), ty: Type::Int }], + vec!["id".to_string()], + None, + ) + .await + .unwrap(); + db.create_table( + "Orders".to_string(), + vec![ + ColumnSpec { name: "id".to_string(), ty: Type::Int }, + ColumnSpec { name: "CustId".to_string(), ty: Type::Int }, + ], + vec!["id".to_string()], + None, + ) + .await + .unwrap(); + db.add_relationship( + None, + "Customers".to_string(), + "id".to_string(), + "Orders".to_string(), + "CustId".to_string(), + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + None, + ) + .await + .unwrap(); + + // Insert into Orders with a CustId that has no parent. + let cmd = Command::Insert { + table: "Orders".to_string(), + columns: Some(vec!["id".to_string(), "CustId".to_string()]), + values: vec![ + Value::Number("1".to_string()), + Value::Number("999".to_string()), + ], + }; + let err = db + .insert( + "Orders".to_string(), + Some(vec!["id".to_string(), "CustId".to_string()]), + vec![ + Value::Number("1".to_string()), + Value::Number("999".to_string()), + ], + None, + ) + .await + .unwrap_err(); + + let facts = enrich_dsl_failure(&db, &cmd, &err).await; + assert_eq!(facts.table.as_deref(), Some("Orders")); + assert_eq!(facts.column.as_deref(), Some("CustId")); + assert_eq!(facts.parent_table.as_deref(), Some("Customers")); + assert_eq!(facts.parent_column.as_deref(), Some("id")); + assert_eq!(facts.value.as_deref(), Some("999")); + // FK pinpoint not implemented in v1. + assert!(facts.diagnostic_table.is_none()); + }); +} + +#[test] +fn enrich_fk_insert_natural_order_multi_value_resolves_via_schema() { + // Regression: `insert into Orders values (4, 11.99)` — + // natural-order multi-value INSERT, no explicit columns, + // and the schema has a serial PK that gets auto-skipped. + // Enrichment must still resolve parent_table / + // parent_column / value via the schema-aware lookup. + let db = db(); + rt().block_on(async { + db.create_table( + "Customers".to_string(), + vec![ColumnSpec { name: "id".to_string(), ty: Type::Int }], + vec!["id".to_string()], + None, + ) + .await + .unwrap(); + db.create_table( + "Orders".to_string(), + vec![ + ColumnSpec { name: "id".to_string(), ty: Type::Serial }, + ColumnSpec { name: "CustId".to_string(), ty: Type::Int }, + ColumnSpec { name: "Total".to_string(), ty: Type::Real }, + ], + vec!["id".to_string()], + None, + ) + .await + .unwrap(); + db.add_relationship( + None, + "Customers".to_string(), + "id".to_string(), + "Orders".to_string(), + "CustId".to_string(), + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + None, + ) + .await + .unwrap(); + + // Natural-order: serial PK auto-fills, so positional + // values map to (CustId, Total). CustId=4 has no + // matching parent → FK violation. + let cmd = Command::Insert { + table: "Orders".to_string(), + columns: None, + values: vec![ + Value::Number("4".to_string()), + Value::Number("11.99".to_string()), + ], + }; + let err = db + .insert( + "Orders".to_string(), + None, + vec![ + Value::Number("4".to_string()), + Value::Number("11.99".to_string()), + ], + None, + ) + .await + .unwrap_err(); + + let facts = enrich_dsl_failure(&db, &cmd, &err).await; + assert_eq!(facts.parent_table.as_deref(), Some("Customers")); + assert_eq!(facts.parent_column.as_deref(), Some("id")); + assert_eq!( + facts.value.as_deref(), + Some("4"), + "natural-order with serial PK skip should map values[0] to CustId" + ); + }); +} + +// ---- FOREIGN KEY (parent-side, DELETE) -------------------------- + +#[test] +fn enrich_fk_delete_resolves_child_table() { + let db = db(); + rt().block_on(async { + db.create_table( + "Customers".to_string(), + vec![ColumnSpec { name: "id".to_string(), ty: Type::Int }], + vec!["id".to_string()], + None, + ) + .await + .unwrap(); + db.create_table( + "Orders".to_string(), + vec![ + ColumnSpec { name: "id".to_string(), ty: Type::Int }, + ColumnSpec { name: "CustId".to_string(), ty: Type::Int }, + ], + vec!["id".to_string()], + None, + ) + .await + .unwrap(); + db.add_relationship( + None, + "Customers".to_string(), + "id".to_string(), + "Orders".to_string(), + "CustId".to_string(), + ReferentialAction::NoAction, + ReferentialAction::NoAction, + false, + None, + ) + .await + .unwrap(); + db.insert( + "Customers".to_string(), + None, + vec![Value::Number("1".to_string())], + None, + ) + .await + .unwrap(); + db.insert( + "Orders".to_string(), + None, + vec![Value::Number("1".to_string()), Value::Number("1".to_string())], + None, + ) + .await + .unwrap(); + + // Delete the parent that has children — engine refuses. + let cmd = Command::Delete { + table: "Customers".to_string(), + filter: RowFilter::Where { + column: "id".to_string(), + value: Value::Number("1".to_string()), + }, + }; + let err = db + .delete( + "Customers".to_string(), + RowFilter::Where { + column: "id".to_string(), + value: Value::Number("1".to_string()), + }, + None, + ) + .await + .unwrap_err(); + + let facts = enrich_dsl_failure(&db, &cmd, &err).await; + assert_eq!(facts.table.as_deref(), Some("Customers")); + assert_eq!(facts.child_table.as_deref(), Some("Orders")); + }); +} + +// ---- non-engine error → empty enrichment ------------------------ + +#[test] +fn enrich_unsupported_returns_default_facts() { + let db = db(); + rt().block_on(async { + let err = DbError::Unsupported("nope".to_string()); + let cmd = Command::DropTable { name: "X".to_string() }; + let facts = enrich_dsl_failure(&db, &cmd, &err).await; + assert!(facts.table.is_none()); + assert!(facts.column.is_none()); + assert!(facts.value.is_none()); + assert!(facts.parent_table.is_none()); + assert!(facts.child_table.is_none()); + assert!(facts.diagnostic_table.is_none()); + }); +} diff --git a/tests/walking_skeleton.rs b/tests/walking_skeleton.rs index b2983ed..3021522 100644 --- a/tests/walking_skeleton.rs +++ b/tests/walking_skeleton.rs @@ -578,6 +578,7 @@ fn dsl_failure_shows_friendly_error_in_output() { message: "no such table: Ghost".to_string(), kind: rdbms_playground::db::SqliteErrorKind::NoSuchTable, }, + facts: rdbms_playground::friendly::FailureContext::default(), }); let rendered = rendered_text(&mut app, &Theme::dark(), 80, 24); assert!(