From f0c0a718605924227076bef7cb8b403c388261b6 Mon Sep 17 00:00:00 2001 From: Sergio Date: Sat, 9 May 2026 22:15:42 +0000 Subject: [PATCH] =?UTF-8?q?feat(nakui-core,nakui-ui):=20FieldOp::Clear=20?= =?UTF-8?q?=E2=80=94=20borrar=20values=20v=C3=ADa=20form=20vac=C3=ADo?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nueva variante semántica del kernel: Clear { path } remueve la key del record, distinta de Set { value: Null } (que deja la key con valor literal null). Habilita "limpiar" un field optional vaciando el input en la UI. nakui-core: - delta::FieldOp::Clear + simulate_on + capability_token (mismo shape que Set: "entity.field"). - MemoryStore::apply_dry_run y apply: Set/Clear comparten pre-condition (record existe + es objeto). Clear de field ausente = no-op silencioso. - SurrealStore: equivalente con `UPDATE ... UNSET `. - Executor capability check: Set/Clear comparten match. - Conservation rules NO consideran Clear (sólo Set) — documentado como morphism-author responsibility. nakui-ui: - commit_seed acumula `to_clear: Vec` con optional empties en lugar de `continue` silencioso. - EDIT branch: nuevo helper compute_clear_fields filtra a sólo los fields con current value non-null. Combina Set + Clear ops. NoChange ahora requiere ambos vacíos. Log entry incluye `cleared: [...]` sólo si non-empty. Updated.changed cuenta sets+clears. Tests: +7 en nakui-core (4 store + 3 delta), +3 en nakui-ui. Suites: 34/34 nakui-core, 40/40 nakui-ui. Workspace build verde. E2E del morphism real intacto. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 96 ++++++++++++++ crates/apps/nakui-ui/src/main.rs | 119 ++++++++++++++---- crates/modules/nakui/core/src/delta.rs | 91 ++++++++++++++ crates/modules/nakui/core/src/executor.rs | 49 ++++---- crates/modules/nakui/core/src/store.rs | 104 ++++++++++++++- .../modules/nakui/core/src/surreal_store.rs | 24 +++- 6 files changed, 438 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c194f8e..26c96f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,102 @@ ratio/diff ver `git show `. ## 2026-05-09 +### feat(nakui-core,nakui-ui): FieldOp::Clear — borrar values vía form vacío +Cierra el último pendiente de UX del round. El edit no podía +"borrar" un value vaciando el input — empty optional fields +hacían `continue` en `commit_seed`, así que el current value +quedaba intacto. Para honrar el intent del usuario ("este field +ya no aplica") necesitábamos un FieldOp explícito que remueva +la key del map. + +Cambios en **nakui-core** (la variante es semántica del kernel, +no específica de la UI): + +- **`delta::FieldOp::Clear { path }`** — nueva variante. + Distinta de `Set { value: Null }`: Clear borra la clave; Set + Null deja la clave con valor literal `null`. Importa para + downstream que diferencia "ausente" vs "presente como null" + (ej: serde con `skip_serializing_if = "Option::is_none"`). +- **`capability_token`** — Clear devuelve `entity.field`, + mismo shape que Set. Una capability `writes: ["Customer.notes"]` + autoriza tanto Set como Clear sobre ese field. +- **`simulate_on`** — Clear remueve la key del Object si el + state es Some(Object). Skip silente si el state es None + (deleted) o no-objeto. +- **`MemoryStore::apply_dry_run`** — Set y Clear comparten + pre-condición (record padre existe + es objeto). Pattern + combinado con `|`. +- **`MemoryStore::apply`** — Clear hace `map.remove(field)`. + Field ausente = no-op silencioso (post-state idéntico). +- **`SurrealStore::apply_dry_run`** — Set/Clear combinados. +- **`SurrealStore::apply`** — Clear emite + `UPDATE type::thing UNSET `. El field name viene de + un FieldSpec validado upstream; SurrealQL no soporta binding + de identifiers, así que va inline (con la advertencia + documentada en el comment). +- **`Executor` capability check** — Set/Clear comparten match + (mismo token shape, misma resolución a binding role). +- **Conservation rules** (en `check_conservation`) NO consideran + Clear — sólo Set. Documentado: morphism authors que querían + clear de un field con conservation tienen que ser cuidadosos; + KCL post-checks pueden capturar violations. + +Cambios en **nakui-ui**: + +- **`commit_seed` loop** acumula `to_clear: Vec` con + los nombres de fields optional empty (en lugar de hacer + `continue` silencioso). +- **EDIT branch**: + - Computa `set_delta` (igual que antes) + `clear_fields` via + nuevo helper `compute_clear_fields(current, to_clear)`. + - Helper filtra a sólo los fields que actualmente tienen + valor non-null — Clear de un field ausente o ya null no + se emite (sería no-op semántico). Preserva el orden del + input para estabilidad del log entry. + - Construye `ops` combinando Set + Clear. + - NoChange ahora requiere AMBOS vacíos (set_delta y + clear_fields). + - `params` del log entry incluye `cleared: ["field1", ...]` + sólo si non-empty (preserva la shape `fields:` para + edits sin clears). + - `CommitOutcome::Updated.changed = sets + clears` para + que el toast `"actualizado X (N campo(s))"` siga siendo + preciso. + +Tests nuevos: +- **delta.rs**: `simulate_clear_removes_field`, + `simulate_clear_then_set_same_field_keeps_set`, + `clear_capability_token_matches_set_shape`. +- **store.rs**: `apply_clear_removes_field_key`, + `apply_clear_on_absent_field_is_noop`, + `dry_run_rejects_clear_on_missing_record`, + `dry_run_rejects_clear_on_non_object`. +- **nakui-ui main.rs**: `clear_fields_skips_absent_and_null`, + `clear_fields_preserves_input_order`, + `clear_fields_empty_when_current_is_null`. + +34 tests verdes en nakui-core (+7), 40 en nakui-ui (+3). +Workspace build verde. E2E del morphism real +`morphism_pipeline_executes_real_sales_vender` intacto — `vender` +no usa Clear. + +Implicaciones: +- **El log puede crecer con entries `ui.edit_record` que sólo + tienen `cleared: [...]`** sin `fields`. Esperado y esperable. +- **Replay**: las entries con Clear se aplican en orden via + `store.apply(&ops)`. La semantic es deterministic. +- **Si un módulo tiene KCL invariants sobre la presencia de un + field**, el usuario podría romper el record vaciando ese + field via UI. Hoy esto NO se chequea — `ui.edit_record` es + un morphism manual que no pasa por `Executor::compute`. Si + esto es un problema, el camino futuro es validar contra el + KCL del entity al submit (otro pendiente). + +Pendientes restantes: +- **Validación cross-field** (ej: UUID del EntityRef existe en + la entity referida). +- **Validación KCL del record post-edit** antes de emitir Set/Clear. + ### feat(nakui-ui): snapshot/compaction durante runtime cada N writes Cierra el último pending del round de persistencia. Antes el compact sólo corría al startup — para una sesión larga con muchas escrituras, diff --git a/crates/apps/nakui-ui/src/main.rs b/crates/apps/nakui-ui/src/main.rs index 2111ecc..0e45482 100644 --- a/crates/apps/nakui-ui/src/main.rs +++ b/crates/apps/nakui-ui/src/main.rs @@ -700,6 +700,11 @@ impl MetaUi { None => return Err("ninguna vista activa".into()), }; let mut obj = serde_json::Map::new(); + // Fields que el form deja vacíos y son optional: candidatos a + // `FieldOp::Clear` en el path de EDIT (sólo se emiten si el + // current store value tiene algo en ese key). En el SEED path + // simplemente no se incluyen, igual que antes. + let mut to_clear: Vec = Vec::new(); for f in &spec_fields { let raw = self .form_inputs @@ -710,6 +715,7 @@ impl MetaUi { return Err(format!("campo '{}' es obligatorio", f.label)); } if raw.is_empty() && !f.required { + to_clear.push(f.name.clone()); continue; } let value = parse_field_value(f.kind, &raw) @@ -724,16 +730,10 @@ impl MetaUi { if let Some((_, id)) = editing_match { // EDIT path: delta-only. Cargar el record actual del store - // y emitir `FieldOp::Set` sólo para los campos cuyo valor - // nuevo difiere del actual. Si nada cambió, ningún log - // entry y ningún apply — el toast lo refleja. - // - // Nota: campos que el form deja vacíos *no* se incluyen - // en `obj` (skip arriba), así que no se pueden "limpiar" - // borrando el input. Esto es consistente con el comportamiento - // pre-delta y con el seed path. Para clearear hay que - // declarar el field como required y forzar un value, o - // implementar un FieldOp::Clear futuro. + // y emitir `FieldOp::Set` por cada campo cuyo valor nuevo + // difiere del actual + `FieldOp::Clear` por cada optional + // empty cuyo current tenía un valor non-null. Si nada + // cambió, ningún log entry y ningún apply. let current: Value = { let store = self .store @@ -741,15 +741,15 @@ impl MetaUi { .map_err(|_| "store mutex envenenado".to_string())?; store.load(entity, id).unwrap_or(Value::Null) }; - let delta = compute_field_delta(¤t, &obj); + let set_delta = compute_field_delta(¤t, &obj); + let clear_fields = compute_clear_fields(¤t, &to_clear); - if delta.is_empty() { - // No-op edit: no entry al log, no apply. Limpia - // editing en el caller via toast diferente. + if set_delta.is_empty() && clear_fields.is_empty() { + // No-op edit: no entry al log, no apply. return Ok(CommitOutcome::NoChange(id)); } - let ops: Vec = delta + let mut ops: Vec = set_delta .iter() .map(|(field, value)| FieldOp::Set { path: FieldPath { @@ -760,21 +760,40 @@ impl MetaUi { value: value.clone(), }) .collect(); + for field in &clear_fields { + ops.push(FieldOp::Clear { + path: FieldPath { + entity: entity.to_string(), + id, + field: field.clone(), + }, + }); + } if let Some(log_arc) = self.event_log.as_ref() { let mut log = log_arc .lock() .map_err(|_| "log mutex envenenado".to_string())?; let seq = log.next_seq(); + let mut params = serde_json::Map::new(); + params.insert("entity".into(), json!(entity)); + params.insert("id".into(), json!(id.to_string())); + if !set_delta.is_empty() { + params.insert("fields".into(), Value::Object(set_delta.clone())); + } + if !clear_fields.is_empty() { + params.insert( + "cleared".into(), + Value::Array( + clear_fields.iter().map(|s| json!(s)).collect(), + ), + ); + } log.append(LogEntry::Morphism { seq, morphism: "ui.edit_record".into(), inputs: Default::default(), - params: json!({ - "entity": entity, - "id": id.to_string(), - "fields": Value::Object(delta.clone()), - }), + params: Value::Object(params), ops: ops.clone(), schema_hash: None, }) @@ -784,10 +803,10 @@ impl MetaUi { .store .lock() .map_err(|_| "store mutex envenenado".to_string())?; - store.apply(&ops).map_err(|e| format!("apply Set: {e}"))?; + store.apply(&ops).map_err(|e| format!("apply edit ops: {e}"))?; Ok(CommitOutcome::Updated { id, - changed: delta.len(), + changed: set_delta.len() + clear_fields.len(), }) } else { // SEED path: alta nueva. @@ -922,6 +941,24 @@ fn maybe_compact_log( ))) } +/// Decide cuáles fields del `to_clear` candidate list ameritan +/// realmente un `FieldOp::Clear`: sólo los que existen en el current +/// con un valor non-null. Para fields ausentes o ya null, Clear es +/// no-op semántico (el post-state es el mismo) y dropearlos +/// preserva la propiedad "1 op = 1 cambio efectivo" del log. +/// +/// Preserva el orden del input para que el log entry sea estable. +fn compute_clear_fields(current: &Value, to_clear: &[String]) -> Vec { + to_clear + .iter() + .filter(|f| match current.get(f.as_str()) { + None | Some(Value::Null) => false, + Some(_) => true, + }) + .cloned() + .collect() +} + /// Calcula el delta entre el record actual y los valores propuestos /// del form. Devuelve un Map con sólo los campos cuyo valor difiere. /// @@ -2022,6 +2059,44 @@ mod tests { let _ = std::fs::remove_file(&snap_path); } + #[test] + fn clear_fields_skips_absent_and_null() { + let current = json!({ + "name": "Acme", + "notes": "lorem", + "tag": null, + }); + let to_clear = vec![ + "name".to_string(), + "notes".to_string(), + "tag".to_string(), + "missing".to_string(), + ]; + let actual = compute_clear_fields(¤t, &to_clear); + assert_eq!( + actual, + vec!["name".to_string(), "notes".to_string()], + "tag (null) y missing (ausente) deberían filtrarse — Clear sería no-op" + ); + } + + #[test] + fn clear_fields_preserves_input_order() { + let current = json!({"a": 1, "b": 2, "c": 3}); + let to_clear = vec!["c".to_string(), "a".to_string(), "b".to_string()]; + let actual = compute_clear_fields(¤t, &to_clear); + assert_eq!(actual, vec!["c", "a", "b"], "orden del input se preserva"); + } + + #[test] + fn clear_fields_empty_when_current_is_null() { + // Record no existe en el store (load → None → Value::Null + // upstream). Ningún clear debería emitirse. + let current = Value::Null; + let to_clear = vec!["name".to_string()]; + assert!(compute_clear_fields(¤t, &to_clear).is_empty()); + } + #[test] fn snapshot_path_for_replaces_extension() { use std::path::Path; diff --git a/crates/modules/nakui/core/src/delta.rs b/crates/modules/nakui/core/src/delta.rs index 35f3faf..2cdf6cc 100644 --- a/crates/modules/nakui/core/src/delta.rs +++ b/crates/modules/nakui/core/src/delta.rs @@ -16,6 +16,17 @@ pub enum FieldOp { path: FieldPath, value: Value, }, + /// Remove a single field key from a record. Distinto de `Set { value: Null }`: + /// `Clear` borra la clave del map; un load posterior no encuentra el + /// campo (`None`/`Value::Null` semantically). `Set Null` por contraste + /// deja la clave con valor literal `null`. La distinción importa para + /// downstream code que diferencia "ausente" de "presente como null" + /// (ej: serialize que `skip_serializing_if = "Option::is_none"`). + /// + /// Capability token: `entity.field` (mismo shape que Set). + Clear { + path: FieldPath, + }, Create { entity: String, id: Uuid, @@ -33,12 +44,87 @@ impl FieldOp { pub fn capability_token(&self) -> String { match self { FieldOp::Set { path, .. } => format!("{}.{}", path.entity, path.field), + FieldOp::Clear { path } => format!("{}.{}", path.entity, path.field), FieldOp::Create { entity, .. } => entity.clone(), FieldOp::Delete { entity, .. } => entity.clone(), } } } +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn simulate_clear_removes_field() { + let id = Uuid::new_v4(); + let state = json!({"name": "Acme", "notes": "lorem"}); + let op = FieldOp::Clear { + path: FieldPath { + entity: "Customer".into(), + id, + field: "notes".into(), + }, + }; + let after = simulate_on(&state, "Customer", id, &[op]).unwrap(); + let map = after.as_object().unwrap(); + assert!(!map.contains_key("notes")); + assert_eq!(map.get("name"), Some(&json!("Acme"))); + } + + #[test] + fn simulate_clear_then_set_same_field_keeps_set() { + let id = Uuid::new_v4(); + let state = json!({"name": "Acme", "notes": "lorem"}); + let ops = vec![ + FieldOp::Clear { + path: FieldPath { + entity: "Customer".into(), + id, + field: "notes".into(), + }, + }, + FieldOp::Set { + path: FieldPath { + entity: "Customer".into(), + id, + field: "notes".into(), + }, + value: json!("nuevo"), + }, + ]; + let after = simulate_on(&state, "Customer", id, &ops).unwrap(); + assert_eq!(after.get("notes"), Some(&json!("nuevo"))); + } + + #[test] + fn clear_capability_token_matches_set_shape() { + let id = Uuid::new_v4(); + let set = FieldOp::Set { + path: FieldPath { + entity: "Customer".into(), + id, + field: "notes".into(), + }, + value: json!("x"), + }; + let clear = FieldOp::Clear { + path: FieldPath { + entity: "Customer".into(), + id, + field: "notes".into(), + }, + }; + assert_eq!(set.capability_token(), "Customer.notes"); + assert_eq!( + clear.capability_token(), + set.capability_token(), + "Clear y Set comparten token shape para el capability check" + ); + } +} + /// Apply only the ops that target `(entity, id)` to `state` and return the /// new value. Returns `None` if a Delete op removes the target — callers /// should skip post-checks against a deleted entity rather than running @@ -52,6 +138,11 @@ pub fn simulate_on(state: &Value, entity: &str, id: Uuid, ops: &[FieldOp]) -> Op map.insert(path.field.clone(), value.clone()); } } + FieldOp::Clear { path } if path.entity == entity && path.id == id => { + if let Some(Value::Object(map)) = s.as_mut() { + map.remove(&path.field); + } + } FieldOp::Create { entity: e, id: i, diff --git a/crates/modules/nakui/core/src/executor.rs b/crates/modules/nakui/core/src/executor.rs index 07627b7..cb3d453 100644 --- a/crates/modules/nakui/core/src/executor.rs +++ b/crates/modules/nakui/core/src/executor.rs @@ -275,28 +275,35 @@ impl Executor { let declared: HashSet<&str> = spec.writes.iter().map(String::as_str).collect(); for op in &ops { let token = match op { - FieldOp::Set { path, .. } => match id_to_input.get(&path.id) { - Some(binding) if binding.entity == path.entity => { - format!("{}.{}", binding.role, path.field) + // Set y Clear comparten el mismo token shape: ambos + // mutan un field específico de un record tracked. + FieldOp::Set { path, .. } | FieldOp::Clear { path } => { + match id_to_input.get(&path.id) { + Some(binding) if binding.entity == path.entity => { + format!("{}.{}", binding.role, path.field) + } + Some(_) => { + return Err(ExecError::CapabilityViolation { + morphism: morphism_name.to_string(), + token: format!( + ".{}.{}", + path.entity, path.field + ), + declared: spec.writes.clone(), + }); + } + None => { + return Err(ExecError::CapabilityViolation { + morphism: morphism_name.to_string(), + token: format!( + ".{}.{}", + path.entity, path.field + ), + declared: spec.writes.clone(), + }); + } } - Some(_) => { - return Err(ExecError::CapabilityViolation { - morphism: morphism_name.to_string(), - token: format!( - ".{}.{}", - path.entity, path.field - ), - declared: spec.writes.clone(), - }); - } - None => { - return Err(ExecError::CapabilityViolation { - morphism: morphism_name.to_string(), - token: format!(".{}.{}", path.entity, path.field), - declared: spec.writes.clone(), - }); - } - }, + } FieldOp::Create { entity, .. } => entity.clone(), FieldOp::Delete { entity, .. } => entity.clone(), }; diff --git a/crates/modules/nakui/core/src/store.rs b/crates/modules/nakui/core/src/store.rs index ef10b68..592182f 100644 --- a/crates/modules/nakui/core/src/store.rs +++ b/crates/modules/nakui/core/src/store.rs @@ -231,7 +231,11 @@ impl Store for MemoryStore { fn apply_dry_run(&self, ops: &[FieldOp]) -> Result<(), StoreError> { for op in ops { match op { - FieldOp::Set { path, .. } => { + FieldOp::Set { path, .. } | FieldOp::Clear { path } => { + // Set y Clear comparten la misma pre-condición: el + // record padre tiene que existir y ser un objeto. + // Clear de un field que no existe en el map es no-op + // benigno en apply (no error). match self.records.get(&path.entity).and_then(|m| m.get(&path.id)) { None => { return Err(StoreError::NotFound(path.entity.clone(), path.id)); @@ -283,6 +287,21 @@ impl Store for MemoryStore { }; map.insert(path.field.clone(), value.clone()); } + FieldOp::Clear { path } => { + let rec = self + .records + .get_mut(&path.entity) + .and_then(|m| m.get_mut(&path.id)) + .expect("validated by dry_run"); + let map = match rec { + Value::Object(m) => m, + _ => unreachable!("dry_run guards against non-object"), + }; + // Clear de un field ausente: no-op silencioso. El + // post-state es el mismo (el field no está) y permite + // que el caller emita Clear sin hacer load previo. + map.remove(&path.field); + } FieldOp::Create { entity, id, data } => { self.records .entry(entity.clone()) @@ -394,6 +413,89 @@ mod tests { assert!(store.apply_dry_run(&[op]).is_ok()); } + #[test] + fn apply_clear_removes_field_key() { + let mut store = MemoryStore::new(); + let id = Uuid::new_v4(); + store.seed( + "Customer", + id, + json!({"id": id.to_string(), "name": "Acme", "notes": "lorem"}), + ); + let op = FieldOp::Clear { + path: FieldPath { + entity: "Customer".into(), + id, + field: "notes".into(), + }, + }; + store.apply(&[op]).unwrap(); + let after = store.load("Customer", id).unwrap(); + let map = after.as_object().unwrap(); + assert!(!map.contains_key("notes"), "notes debería estar borrado"); + assert_eq!(map.get("name"), Some(&json!("Acme")), "otros fields intactos"); + } + + #[test] + fn apply_clear_on_absent_field_is_noop() { + let mut store = MemoryStore::new(); + let id = Uuid::new_v4(); + store.seed( + "Customer", + id, + json!({"id": id.to_string(), "name": "Acme"}), + ); + let op = FieldOp::Clear { + path: FieldPath { + entity: "Customer".into(), + id, + field: "notes".into(), + }, + }; + // No debería errar: clear de un field ausente es benigno. + store.apply(&[op]).unwrap(); + let after = store.load("Customer", id).unwrap(); + assert_eq!( + after.as_object().unwrap().get("name"), + Some(&json!("Acme")) + ); + } + + #[test] + fn dry_run_rejects_clear_on_missing_record() { + let store = MemoryStore::new(); + let id = Uuid::new_v4(); + let op = FieldOp::Clear { + path: FieldPath { + entity: "Customer".into(), + id, + field: "notes".into(), + }, + }; + assert!(matches!( + store.apply_dry_run(&[op]), + Err(StoreError::NotFound(_, _)) + )); + } + + #[test] + fn dry_run_rejects_clear_on_non_object() { + let mut store = MemoryStore::new(); + let id = Uuid::new_v4(); + store.seed("Customer", id, json!(42)); // not an object + let op = FieldOp::Clear { + path: FieldPath { + entity: "Customer".into(), + id, + field: "notes".into(), + }, + }; + assert!(matches!( + store.apply_dry_run(&[op]), + Err(StoreError::NotAnObject(_, _)) + )); + } + #[test] fn iter_returns_canonical_order_regardless_of_insertion() { let a = Uuid::new_v4(); diff --git a/crates/modules/nakui/core/src/surreal_store.rs b/crates/modules/nakui/core/src/surreal_store.rs index c233e6e..6805bff 100644 --- a/crates/modules/nakui/core/src/surreal_store.rs +++ b/crates/modules/nakui/core/src/surreal_store.rs @@ -168,7 +168,11 @@ impl Store for SurrealStore { self.runtime.block_on(async { for op in ops { match op { - FieldOp::Set { path, .. } => { + FieldOp::Set { path, .. } | FieldOp::Clear { path } => { + // Set y Clear comparten la misma pre-condición: + // el record padre tiene que existir. Clear de + // un field inexistente es no-op benigno (UNSET + // sobre un field ausente no falla). let exists = self.exists(&path.entity, path.id).await?; if !exists { return Err(StoreError::NotFound( @@ -360,6 +364,24 @@ impl Store for SurrealStore { .and_then(|r| r.check()) .map_err(map_err)?; } + FieldOp::Clear { path } => { + // SurrealQL `UNSET` borra la key. El field name + // viene de un FieldSpec validado upstream y + // SurrealQL no soporta binding de identifiers + // (sólo valores), así que va inline. Si en + // el futuro se permite que el field name venga + // de un input no-trusted, validar aquí. + self.db + .query(format!( + "UPDATE type::thing($table, $id) UNSET {}", + path.field + )) + .bind(("table", path.entity.clone())) + .bind(("id", path.id.to_string())) + .await + .and_then(|r| r.check()) + .map_err(map_err)?; + } FieldOp::Create { entity, id, data } => { let stripped = strip_app_id(data.clone()); let map = json_to_map(stripped)?;