From e7fa061e234f3e8406092b3034f8bd5556504825 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Sun, 6 Sep 2026 06:25:04 +0300 Subject: [PATCH] Fix scoped semantic drafts and dependent review controls --- crates/oa-desktop/src/forms.rs | 181 +++++++++--- crates/oa-desktop/src/forms/data_fields.rs | 261 +++++++++++++++++- crates/oa-desktop/src/forms/port_fields.rs | 4 +- crates/oa-desktop/src/semantic_review.rs | 130 +++++---- .../oa-desktop/src/semantic_review/tests.rs | 204 ++++++++++++++ crates/oa-desktop/src/semantic_review/view.rs | 24 +- harness/release-semantic-gui.md | 25 +- 7 files changed, 730 insertions(+), 99 deletions(-) diff --git a/crates/oa-desktop/src/forms.rs b/crates/oa-desktop/src/forms.rs index f9a571ba..53695aef 100644 --- a/crates/oa-desktop/src/forms.rs +++ b/crates/oa-desktop/src/forms.rs @@ -45,6 +45,7 @@ pub struct Draft { baseline: Value, pub inputs: BTreeMap, pub exact_strings: bool, + context: Option, port_contract_drafts: BTreeMap, pub value_browsers: BTreeMap, } @@ -83,6 +84,7 @@ impl Draft { value, inputs: BTreeMap::new(), exact_strings: false, + context: None, port_contract_drafts: BTreeMap::new(), value_browsers: BTreeMap::new(), }; @@ -142,6 +144,16 @@ impl Draft { pub fn materialize(&self) -> Result { self.materialize_inputs(|_| true) } + /// Read-only context for dependent controls. Workflow proposals never replace + /// the authored value or its input buffers and are not used by command(). + pub fn context_value(&self) -> Value { + self.context + .clone() + .unwrap_or_else(|| self.materialize().unwrap_or_else(|_| self.value.clone())) + } + pub fn set_context(&mut self, context: Option) { + self.context = context; + } /// Materialize selected scalar subtrees without parsing retained, unselected drafts. pub fn materialize_scopes(&self, scopes: &[String]) -> Result { self.materialize_inputs(|path| { @@ -226,6 +238,97 @@ impl Draft { self.rebuild(); Ok(()) } + /// Replace a workflow-authorized subtree without parsing unrelated inputs, + /// including an invalid value which this replacement deliberately removes. + pub fn shape_scoped(&mut self, path: &str, value: Value) -> Result<(), String> { + *self + .value + .pointer_mut(path) + .ok_or("Property no longer exists.")? = value; + self.rebuild_retaining(|old| (!within(old, path)).then(|| old.to_owned())); + Ok(()) + } + fn rebuild_retaining(&mut self, new_path: impl Fn(&str) -> Option) { + let previous = std::mem::take(&mut self.inputs); + self.rebuild(); + for (path, input) in previous { + if let Some(path) = new_path(&path) + && self.inputs.contains_key(&path) + { + self.inputs.insert(path, input); + } + } + } + pub fn array_scoped( + &mut self, + path: &str, + index: Option, + direction: i32, + ) -> Result<(), String> { + let mut candidate = self.value.clone(); + let count = candidate + .pointer(path) + .and_then(Value::as_array) + .ok_or("List no longer exists.")? + .len(); + edit_array(&mut candidate, path, index, direction)?; + self.value = candidate; + self.rebuild_retaining(|old| { + let Some(tail) = old.strip_prefix(&format!("{path}/")) else { + return Some(old.to_owned()); + }; + let (number, rest) = tail.split_once('/').unwrap_or((tail, "")); + let position = number.parse::().ok()?; + let position = match index { + Some(index) if direction == 0 && position == index => return None, + Some(index) if direction == 0 && position > index => position - 1, + Some(index) if direction != 0 => { + let target = (index as i32 + direction).clamp(0, count as i32 - 1) as usize; + if position == index { + target + } else if position == target { + index + } else { + position + } + } + _ => position, + }; + Some(if rest.is_empty() { + format!("{path}/{position}") + } else { + format!("{path}/{position}/{rest}") + }) + }); + Ok(()) + } + pub fn append_scoped(&mut self, path: &str, value: Value) -> Result<(), String> { + self.value + .pointer_mut(path) + .and_then(Value::as_array_mut) + .ok_or("List no longer exists.")? + .push(value); + self.rebuild_retaining(|path| Some(path.to_owned())); + Ok(()) + } + pub fn value_path_scoped( + &mut self, + path: &str, + key: crate::data_values::FieldPathKey, + ) -> Result<(), String> { + let row = self + .value + .pointer_mut(path) + .and_then(Value::as_object_mut) + .ok_or("Message value no longer exists.")?; + row.insert("path".into(), json!(key.path)); + row.insert("field".into(), json!(key.field)); + self.rebuild_retaining(|old| { + (!within(old, &format!("{path}/path")) && !within(old, &format!("{path}/field"))) + .then(|| old.to_owned()) + }); + Ok(()) + } pub fn array( &mut self, path: &str, @@ -233,36 +336,7 @@ impl Draft { direction: i32, ) -> Result<(), String> { let mut candidate = self.materialize()?; - let items = candidate - .pointer_mut(path) - .and_then(Value::as_array_mut) - .ok_or("List no longer exists.")?; - match index { - None => items.push(if path.ends_with("literals") { - json!("New value") - } else if path.ends_with("bindings") { - json!({"referenced": Id::nil(), "local": Id::nil()}) - } else if path.ends_with("values") { - json!({"path": [], "field": Id::nil(), "expression": ""}) - } else if path.ends_with("routes") { - json!({"edge": Id::nil(), "waypoints": [], "label": null}) - } else if path.ends_with("waypoints") { - json!({"x": 0.0, "y": 0.0}) - } else if path.ends_with("port_anchors") { - json!({"port": Id::nil(), "side": "Right", "offset": 0.5}) - } else { - json!(Id::nil()) - }), - Some(i) if i < items.len() => { - if direction == 0 { - items.remove(i); - } else { - let target = (i as i32 + direction).clamp(0, items.len() as i32 - 1) as usize; - items.swap(i, target); - } - } - _ => return Err("List item no longer exists.".into()), - } + edit_array(&mut candidate, path, index, direction)?; self.value = candidate; self.rebuild(); Ok(()) @@ -364,6 +438,51 @@ impl Draft { } } +fn within(path: &str, root: &str) -> bool { + path == root + || path + .strip_prefix(root) + .is_some_and(|tail| tail.starts_with('/')) +} +fn edit_array( + value: &mut Value, + path: &str, + index: Option, + direction: i32, +) -> Result<(), String> { + let items = value + .pointer_mut(path) + .and_then(Value::as_array_mut) + .ok_or("List no longer exists.")?; + match index { + None => items.push(if path.ends_with("literals") { + json!("New value") + } else if path.ends_with("bindings") { + json!({"referenced": Id::nil(), "local": Id::nil()}) + } else if path.ends_with("values") { + json!({"path": [], "field": Id::nil(), "expression": ""}) + } else if path.ends_with("routes") { + json!({"edge": Id::nil(), "waypoints": [], "label": null}) + } else if path.ends_with("waypoints") { + json!({"x": 0.0, "y": 0.0}) + } else if path.ends_with("port_anchors") { + json!({"port": Id::nil(), "side": "Right", "offset": 0.5}) + } else { + json!(Id::nil()) + }), + Some(i) if i < items.len() => { + if direction == 0 { + items.remove(i); + } else { + let target = (i as i32 + direction).clamp(0, items.len() as i32 - 1) as usize; + items.swap(i, target); + } + } + _ => return Err("List item no longer exists.".into()), + } + Ok(()) +} + fn friendly(name: &str) -> String { match name { "id" => "UUID identity".into(), @@ -578,7 +697,7 @@ fn reference_choices(draft: &Draft, project: &Project, path: &str, current: Id) } fn reference_ids(draft: &Draft, project: &Project, path: &str) -> Vec { - let value = draft.materialize().unwrap_or_else(|_| draft.value.clone()); + let value = draft.context_value(); if path.starts_with("/port_anchors/") && path.ends_with("/port") { let owner = value .get("element") diff --git a/crates/oa-desktop/src/forms/data_fields.rs b/crates/oa-desktop/src/forms/data_fields.rs index 2f87a22d..a00e35e4 100644 --- a/crates/oa-desktop/src/forms/data_fields.rs +++ b/crates/oa-desktop/src/forms/data_fields.rs @@ -3,7 +3,7 @@ use super::*; use iced::widget::column; fn element(draft: &Draft, project: &Project) -> Option { - serde_json::from_value(draft.materialize().unwrap_or_else(|_| draft.value.clone())) + serde_json::from_value(draft.context_value()) .ok() .or_else(|| project.elements.get(&draft.original).cloned()) } @@ -113,9 +113,9 @@ pub(super) fn property<'a>( let minimum_path = path.replace("maximum", "minimum"); json!( draft - .materialize() - .ok() - .and_then(|v| v.pointer(&minimum_path).and_then(Value::as_u64)) + .context_value() + .pointer(&minimum_path) + .and_then(Value::as_u64) .unwrap_or(1) .max(1) ) @@ -311,7 +311,7 @@ fn message_value<'a>( value: &'a Value, path: &str, ) -> View<'a, Message> { - let edited = draft.materialize().unwrap_or_else(|_| draft.value.clone()); + let edited = draft.context_value(); let row = edited.pointer(path).unwrap_or(value).clone(); let current = serde_json::from_value::(row.clone()).ok(); let owner = element(draft, project); @@ -513,6 +513,257 @@ fn field_page( mod tests { use super::*; + fn semantic_state(app: &crate::app::App) -> &crate::semantic_review::State { + let Some(crate::workflow::Workflow::Semantic(state)) = &app.workflow else { + panic!() + }; + state + } + fn semantic_send(app: &mut crate::app::App, event: crate::semantic_review::Event) { + let _ = app.update(Message::Workflow(crate::workflow::Event::Semantic(event))); + } + fn semantic_decide(app: &mut crate::app::App, facet: FieldFacet, decision: SemanticDecision) { + use crate::semantic_review::Event; + semantic_send(app, Event::Select(FacetSelector::Field(facet))); + semantic_send( + app, + Event::Decide(FacetSelector::Field(facet), Some(decision)), + ); + } + fn default_buttons(draft: &Draft, project: &Project, path: &str) -> usize { + use iced::advanced::widget::{Tree, tree::Tag}; + let button: View<'_, Message> = button(text("Button")).into(); + let tag = Tree::new(button.as_widget()).tag; + let view = property(draft, project, draft.value.pointer(path).unwrap(), path).unwrap(); + fn count(tree: &Tree, tag: Tag) -> usize { + usize::from(tree.tag == tag) + + tree + .children + .iter() + .map(|child| count(child, tag)) + .sum::() + } + count(&Tree::new(view.as_widget()), tag) + } + + #[test] + fn semantic_reference_choices_follow_selected_mechanism_and_revert_on_keep() { + use crate::semantic_review::{Event, State}; + let mut p = Project::empty(); + let oa = p + .add_element(Kind::ExchangeItem, Layer::Oa, "Source", None) + .unwrap(); + let sa = p + .add_element(Kind::ExchangeItem, Layer::Sa, "Target", None) + .unwrap(); + let operation = p + .add_element(Kind::ExchangeItem, Layer::Sa, "Operation base", None) + .unwrap(); + let flow = p + .add_element(Kind::ExchangeItem, Layer::Sa, "Flow base", None) + .unwrap(); + for (id, next) in [ + (oa, ExchangeMechanism::Operation), + (sa, ExchangeMechanism::Flow), + (operation, ExchangeMechanism::Operation), + (flow, ExchangeMechanism::Flow), + ] { + let Some(SharedBody::ExchangeItem { mechanism, .. }) = + p.elements.get_mut(&id).unwrap().shared_mut() + else { + panic!() + }; + *mechanism = next; + } + let trace = Id::new_v4(); + p.traces.insert( + trace, + Trace { + id: trace, + alias: "item_trace".into(), + name: "Review item".into(), + description: String::new(), + oa, + sa, + kind: TraceKind::Manual, + baseline: None, + }, + ); + let mut state = State::new(&p, oa).unwrap(); + let facet = FacetSelector::ExchangeItem(ExchangeItemFacet::Mechanism); + let path = "/body/Sa/Shared/ExchangeItem/supertype"; + let raw = state.draft.as_ref().unwrap().value.clone(); + state.update( + &p, + Event::Decide(facet.clone(), Some(SemanticDecision::TakeSource)), + ); + assert!(state.can_apply(), "{:?}", state.error); + let choices = reference_ids(state.draft.as_ref().unwrap(), &p, path); + assert!(choices.contains(&operation)); + assert!(!choices.contains(&flow)); + assert_eq!(state.draft.as_ref().unwrap().value, raw); + state.update(&p, Event::Decide(facet, Some(SemanticDecision::KeepTarget))); + let choices = reference_ids(state.draft.as_ref().unwrap(), &p, path); + assert!(!choices.contains(&operation)); + assert!(choices.contains(&flow)); + } + + #[test] + fn independent_review_combined_take_type_informs_actual_default_form_in_both_directions() { + use crate::semantic_review::Event; + for reverse in [false, true] { + for decision in [SemanticDecision::TakeSource, SemanticDecision::Candidate] { + let mut p = Project::empty(); + let mut pair = vec![]; + for layer in [Layer::Oa, Layer::Sa] { + let source = (layer == Layer::Oa) != reverse; + let record = p + .add_element(Kind::DataType, layer, "Record", None) + .unwrap(); + *p.elements.get_mut(&record).unwrap().shared_mut().unwrap() = + SharedBody::DataType { + definition: DataTypeDefinition::Record { supertype: None }, + default: None, + unit: String::new(), + }; + let field = p + .add_element(Kind::DataField, layer, "Payload", Some(record)) + .unwrap(); + *p.elements.get_mut(&field).unwrap().shared_mut().unwrap() = + SharedBody::DataField { + type_ref: if source { + TypeRef::Boolean + } else { + TypeRef::Integer + }, + minimum: 0, + maximum: Some(1), + unit: String::new(), + direction: ParameterDirection::Unspecified, + storage: FieldStorage::Value, + default: None, + }; + pair.push(field); + } + let trace = Id::new_v4(); + p.traces.insert( + trace, + Trace { + id: trace, + alias: "payload_trace".into(), + name: "Payload review".into(), + description: String::new(), + oa: pair[0], + sa: pair[1], + kind: TraceKind::Manual, + baseline: None, + }, + ); + let (source, target, layer) = if reverse { + (pair[1], pair[0], "Oa") + } else { + (pair[0], pair[1], "Sa") + }; + let root = format!("/body/{layer}/Shared/DataField"); + let default = format!("{root}/default"); + let type_path = format!("{root}/type_ref"); + let minimum = format!("{root}/minimum"); + let mut app = crate::app::App::with_project(p); + let before = app.document.snapshot(); + let _ = app.update(Message::StartSemanticReview(source)); + // An unfinished candidate must not make the context fall back to + // the old target type, or contaminate a later Keep decision. + semantic_decide(&mut app, FieldFacet::Minimum, SemanticDecision::Candidate); + semantic_send(&mut app, Event::Input(minimum.clone(), "-".into())); + semantic_decide(&mut app, FieldFacet::Type, decision); + if decision == SemanticDecision::Candidate { + semantic_send( + &mut app, + Event::Variant(type_path.clone(), json!("Boolean")), + ); + } + semantic_decide(&mut app, FieldFacet::Default, SemanticDecision::Candidate); + assert!(!semantic_state(&app).can_apply()); + let draft = semantic_state(&app).draft.as_ref().unwrap(); + assert!(matches!( + element(draft, app.project()).unwrap().shared(), + Some(SharedBody::DataField { + type_ref: TypeRef::Boolean, + .. + }) + )); + assert_eq!(draft.inputs[&minimum].raw, "-"); + semantic_decide(&mut app, FieldFacet::Minimum, SemanticDecision::KeepTarget); + semantic_decide(&mut app, FieldFacet::Default, SemanticDecision::Candidate); + assert!(semantic_state(&app).can_apply()); + let draft = semantic_state(&app).draft.as_ref().unwrap(); + assert_eq!(default_buttons(draft, app.project(), &default), 2); + // These are the same element/definition/literal helpers used by + // the visible Set literal default and Add literal buttons. + let owner = element(draft, app.project()).unwrap(); + let Some(SharedBody::DataField { type_ref, .. }) = owner.shared() else { + panic!() + }; + let initial = literal(&definition(app.project(), type_ref).unwrap()).unwrap(); + assert_eq!(initial, json!({"Boolean": false})); + if decision == SemanticDecision::TakeSource { + assert_eq!(draft.value.pointer(&type_path), Some(&json!("Integer"))); + } + // Reverting the type changes the controls without changing its + // retained candidate. Storage constraints likewise read context. + semantic_decide(&mut app, FieldFacet::Type, SemanticDecision::KeepTarget); + assert!(matches!( + element(semantic_state(&app).draft.as_ref().unwrap(), app.project()) + .unwrap() + .shared(), + Some(SharedBody::DataField { + type_ref: TypeRef::Integer, + .. + }) + )); + semantic_decide(&mut app, FieldFacet::Type, decision); + semantic_decide(&mut app, FieldFacet::Storage, SemanticDecision::Candidate); + semantic_send( + &mut app, + Event::Variant(format!("{root}/storage"), json!("Reference")), + ); + assert_eq!( + default_buttons( + semantic_state(&app).draft.as_ref().unwrap(), + app.project(), + &default + ), + 0 + ); + semantic_decide(&mut app, FieldFacet::Storage, SemanticDecision::KeepTarget); + assert_eq!( + default_buttons( + semantic_state(&app).draft.as_ref().unwrap(), + app.project(), + &default + ), + 2 + ); + semantic_decide(&mut app, FieldFacet::Default, SemanticDecision::Candidate); + semantic_send(&mut app, Event::Shape(default, json!([initial]))); + assert!( + semantic_state(&app).can_apply(), + "{:?}", + semantic_state(&app).error + ); + assert_eq!(app.document.bundle(), &before.bundle); + let _ = app.update(Message::Workflow(crate::workflow::Event::Apply)); + assert!( + matches!(app.project().elements[&target].shared(), Some(SharedBody::DataField { + type_ref: TypeRef::Boolean, default: Some(values), minimum: 0, storage: FieldStorage::Value, .. + }) if values == &vec![DataValue::Boolean(false)]) + ); + let _ = app.update(Message::Undo); + assert_eq!(app.document.bundle(), &before.bundle); + } + } + } + #[test] fn selecting_qualified_path_preserves_expression_typed_before_the_next_render() { let (mut f, next) = crate::data_values::tests::recursive_payload(); diff --git a/crates/oa-desktop/src/forms/port_fields.rs b/crates/oa-desktop/src/forms/port_fields.rs index f9b0e77b..5dee2c9f 100644 --- a/crates/oa-desktop/src/forms/port_fields.rs +++ b/crates/oa-desktop/src/forms/port_fields.rs @@ -42,7 +42,7 @@ pub(super) fn variants( _value: &Value, ) -> Option> { if path.ends_with("/Port/contract") { - let current = draft.materialize().unwrap_or_else(|_| draft.value.clone()); + let current = draft.context_value(); let sa = current.pointer("/body/Sa").is_some(); if !sa { return Some(vec![]); @@ -80,7 +80,7 @@ pub(super) fn variants( ); } if path.ends_with("/Port/contract/Component/orientation") { - let current = draft.materialize().unwrap_or_else(|_| draft.value.clone()); + let current = draft.context_value(); let kind_path = path.strip_suffix("orientation").unwrap().to_owned() + "kind"; let orientations: &[&str] = if current.pointer(&kind_path).and_then(Value::as_str) == Some("Standard") { diff --git a/crates/oa-desktop/src/semantic_review.rs b/crates/oa-desktop/src/semantic_review.rs index b0335746..229cbe10 100644 --- a/crates/oa-desktop/src/semantic_review.rs +++ b/crates/oa-desktop/src/semantic_review.rs @@ -184,22 +184,28 @@ impl State { d.edit_text(&path, action); Ok(()) }), - Event::Shape(path, value) => self.edit(project, &path, |d| d.shape(&path, value)), + Event::Shape(path, value) => { + self.edit(project, &path, |d| d.shape_scoped(&path, value)) + } Event::Variant(path, value) => self.edit(project, &path, |d| { - if d.materialize()? + if d.value .pointer(&path) .is_some_and(|v| crate::forms::same_variant(v, &value)) { Ok(()) } else { - d.shape(&path, value) + d.shape_scoped(&path, value) } }), Event::Array(path, index, direction) => { - self.edit(project, &path, |d| d.array(&path, index, direction)) + self.edit(project, &path, |d| d.array_scoped(&path, index, direction)) + } + Event::Append(path, value) => { + self.edit(project, &path, |d| d.append_scoped(&path, value)) + } + Event::ValuePath(path, key) => { + self.edit(project, &path, |d| d.value_path_scoped(&path, key)) } - Event::Append(path, value) => self.edit(project, &path, |d| d.append(&path, value)), - Event::ValuePath(path, key) => self.edit(project, &path, |d| d.value_path(&path, key)), Event::Tab(tab) => { self.tab = tab; replan = false; @@ -360,53 +366,7 @@ impl State { .values() .any(|d| *d == SemanticDecision::Candidate) { - let original = - Draft::from_project(project, target).ok_or("Target has no editable record.")?; - let mut final_value = original.value; - let scopes: Vec<_> = self - .decisions - .iter() - .filter(|(_, decision)| **decision == SemanticDecision::Candidate) - .filter_map(|(facet, _)| scalar::binding(project, target, facet)) - .collect(); - let draft = self - .draft - .as_ref() - .ok_or("Target draft is unavailable.")? - .materialize_scopes(&scopes)?; - let mut map = BTreeMap::new(); - for (oa, sa) in &self.references { - let (from, to) = if self.direction == TransitionDirection::OaToSa { - (*oa, *sa) - } else { - (*sa, *oa) - }; - if map.insert(from, to).is_some_and(|old| old != to) { - return Err("Reverse reference correspondence is ambiguous.".into()); - } - } - map.insert(source, target); - for (facet, decision) in &self.decisions { - if *decision == SemanticDecision::KeepTarget { - continue; - } - let path = scalar::binding(project, target, facet) - .ok_or("Selected property is not applicable to the target.")?; - let value = match decision { - SemanticDecision::KeepTarget => continue, - SemanticDecision::Candidate => draft - .pointer(&path) - .cloned() - .ok_or("Candidate property is no longer available.")?, - SemanticDecision::TakeSource => { - let fact = capture_facet(project, trace.oa, trace.sa, facet)?; - scalar::source_value(&fact, self.direction.source_layer(), &map)? - } - }; - *final_value - .pointer_mut(&path) - .ok_or("Final target property is unavailable.")? = value; - } + let final_value = self.combined_value(project, false)?; let candidate = if project.elements.contains_key(&target) { SemanticRecord::Element( serde_json::from_value(final_value) @@ -423,10 +383,74 @@ impl State { Ok(request) } + /// Compose selected scalar values for both the candidate and dependent form + /// controls. Incomplete inputs retain their stored shape only for controls; + /// request construction always requires every selected input to parse. + fn combined_value(&self, project: &Project, form_context: bool) -> Result { + let trace = self + .trace + .and_then(|id| project.traces.get(&id)) + .ok_or("Choose a trace.")?; + let (source, target) = if self.direction == TransitionDirection::OaToSa { + (trace.oa, trace.sa) + } else { + (trace.sa, trace.oa) + }; + let mut final_value = Draft::from_project(project, target) + .ok_or("Target has no editable record.")? + .value; + let draft = self.draft.as_ref().ok_or("Target draft is unavailable.")?; + let mut map = BTreeMap::new(); + for (oa, sa) in &self.references { + let (from, to) = if self.direction == TransitionDirection::OaToSa { + (*oa, *sa) + } else { + (*sa, *oa) + }; + if map.insert(from, to).is_some_and(|old| old != to) { + return Err("Reverse reference correspondence is ambiguous.".into()); + } + } + map.insert(source, target); + for (facet, decision) in &self.decisions { + if *decision == SemanticDecision::KeepTarget { + continue; + } + let path = scalar::binding(project, target, facet) + .ok_or("Selected property is not applicable to the target.")?; + let value = match decision { + SemanticDecision::KeepTarget => continue, + SemanticDecision::Candidate => { + let value = match draft.materialize_scopes(std::slice::from_ref(&path)) { + Ok(value) => value, + Err(_) if form_context => draft.value.clone(), + Err(error) => return Err(error), + }; + value + .pointer(&path) + .cloned() + .ok_or("Candidate property is no longer available.")? + } + SemanticDecision::TakeSource => { + let fact = capture_facet(project, trace.oa, trace.sa, facet)?; + scalar::source_value(&fact, self.direction.source_layer(), &map)? + } + }; + *final_value + .pointer_mut(&path) + .ok_or("Final target property is unavailable.")? = value; + } + Ok(final_value) + } + pub fn refresh(&mut self, project: &Project) { self.plan = None; self.proposed = None; self.error = None; + let context = self.combined_value(project, true).ok(); + if let Some(draft) = &mut self.draft { + draft.set_context(context); + } let Some(trace) = self.trace.and_then(|id| project.traces.get(&id)) else { self.error = Some( "Several traces are available. Choose the correspondence and review history above." diff --git a/crates/oa-desktop/src/semantic_review/tests.rs b/crates/oa-desktop/src/semantic_review/tests.rs index e864f89c..788f8eb3 100644 --- a/crates/oa-desktop/src/semantic_review/tests.rs +++ b/crates/oa-desktop/src/semantic_review/tests.rs @@ -58,6 +58,210 @@ fn start(app: &mut App, source: Id) { fn facet(value: DataFacet) -> FacetSelector { FacetSelector::Data(value) } + +#[test] +fn independent_review_clear_bound_ignores_retained_invalid_other_facet() { + let (p, oa, sa, _) = fixture(); + let mut app = App::with_project(p); + let before = app.document.snapshot(); + start(&mut app, oa); + let minimum = "/body/Sa/Shared/DataType/definition/Integer/minimum"; + let maximum = "/body/Sa/Shared/DataType/definition/Integer/maximum"; + decide( + &mut app, + facet(DataFacet::IntegerMinimum), + SemanticDecision::Candidate, + ); + send(&mut app, Event::Input(minimum.into(), "-".into())); + decide( + &mut app, + facet(DataFacet::IntegerMinimum), + SemanticDecision::KeepTarget, + ); + decide( + &mut app, + facet(DataFacet::IntegerMaximum), + SemanticDecision::Candidate, + ); + assert!(state(&app).can_apply()); + // Exact event emitted by the upper bound's visible Clear bound button. + send(&mut app, Event::Shape(maximum.into(), Value::Null)); + assert!(state(&app).can_apply(), "{:?}", state(&app).error); + assert_eq!(state(&app).draft.as_ref().unwrap().inputs[minimum].raw, "-"); + assert!( + state(&app) + .draft + .as_ref() + .unwrap() + .value + .pointer(maximum) + .unwrap() + .is_null() + ); + // Clear must also discard the selected invalid value, without requiring it + // to parse before removal. A valid but uncommitted neighboring input survives. + send(&mut app, Event::Shape(maximum.into(), json!(30))); + send(&mut app, Event::Input(maximum.into(), "29".into())); + decide( + &mut app, + facet(DataFacet::IntegerMinimum), + SemanticDecision::Candidate, + ); + assert!(!state(&app).can_apply()); + send(&mut app, Event::Shape(minimum.into(), Value::Null)); + assert!(state(&app).can_apply(), "{:?}", state(&app).error); + assert_eq!( + state(&app).draft.as_ref().unwrap().inputs[maximum].raw, + "29" + ); + assert!( + !state(&app) + .draft + .as_ref() + .unwrap() + .inputs + .contains_key(minimum) + ); + assert_eq!(app.document.bundle(), &before.bundle); + apply(&mut app); + assert!(matches!( + app.project().elements[&sa].shared(), + Some(SharedBody::DataType { + definition: DataTypeDefinition::Integer { + minimum: None, + maximum: Some(29) + }, + .. + }) + )); + let _ = app.update(Message::Undo); + assert_eq!(app.document.bundle(), &before.bundle); +} + +#[test] +fn scoped_literal_list_edits_preserve_unfinished_values_and_other_facet_buffers() { + let mut p = Project::empty(); + let mut pair = vec![]; + for layer in [Layer::Oa, Layer::Sa] { + let item = p + .add_element(Kind::ExchangeItem, layer, "Payload", None) + .unwrap(); + let field = p + .add_element(Kind::DataField, layer, "Numbers", Some(item)) + .unwrap(); + *p.elements.get_mut(&field).unwrap().shared_mut().unwrap() = SharedBody::DataField { + type_ref: TypeRef::Integer, + minimum: 0, + maximum: None, + unit: String::new(), + direction: ParameterDirection::Unspecified, + storage: FieldStorage::Value, + default: Some(vec![DataValue::Integer(1), DataValue::Integer(2)]), + }; + pair.push(field); + } + add_trace(&mut p, pair[0], pair[1], "Numbers review"); + let mut app = App::with_project(p); + start(&mut app, pair[0]); + let root = "/body/Sa/Shared/DataField"; + let minimum = format!("{root}/minimum"); + let default = format!("{root}/default"); + let first = format!("{default}/0/Integer"); + let second = format!("{default}/1/Integer"); + decide( + &mut app, + FacetSelector::Field(FieldFacet::Minimum), + SemanticDecision::Candidate, + ); + send(&mut app, Event::Input(minimum.clone(), "-".into())); + decide( + &mut app, + FacetSelector::Field(FieldFacet::Minimum), + SemanticDecision::KeepTarget, + ); + decide( + &mut app, + FacetSelector::Field(FieldFacet::Default), + SemanticDecision::Candidate, + ); + send(&mut app, Event::Input(first.clone(), "-".into())); + send(&mut app, Event::Input(second.clone(), "23".into())); + send( + &mut app, + Event::Append(default.clone(), json!({"Integer": 3})), + ); + assert_eq!(state(&app).draft.as_ref().unwrap().inputs[&first].raw, "-"); + assert_eq!( + state(&app).draft.as_ref().unwrap().inputs[&second].raw, + "23" + ); + // Same-variant reselection must not throw away the invalid authored value. + send( + &mut app, + Event::Variant(format!("{default}/0"), json!({"Integer": 0})), + ); + assert_eq!(state(&app).draft.as_ref().unwrap().inputs[&first].raw, "-"); + send(&mut app, Event::Array(default.clone(), Some(0), 1)); + assert_eq!(state(&app).draft.as_ref().unwrap().inputs[&first].raw, "23"); + assert_eq!(state(&app).draft.as_ref().unwrap().inputs[&second].raw, "-"); + assert!(!state(&app).can_apply()); + send(&mut app, Event::Array(default, Some(1), 0)); + assert!(state(&app).can_apply(), "{:?}", state(&app).error); + assert_eq!( + state(&app).draft.as_ref().unwrap().inputs[&minimum].raw, + "-" + ); + apply(&mut app); + assert!( + matches!(app.project().elements[&pair[1]].shared(), Some(SharedBody::DataField { + default: Some(values), minimum: 0, .. + }) if values == &vec![DataValue::Integer(23), DataValue::Integer(3)]) + ); +} + +#[test] +fn independent_review_keyed_fact_titles_keep_live_deleted_and_qualified_identity() { + let mut p = Project::empty(); + let a = Id::new_v4(); + let b = Id::new_v4(); + let first = FacetSelector::Child(KeyPair { + oa: Some(a), + sa: None, + }); + let second = FacetSelector::Child(KeyPair { + oa: Some(b), + sa: None, + }); + let first_title = super::view::title(&p, &first); + let second_title = super::view::title(&p, &second); + assert_ne!(first_title, second_title); + assert!(first_title.contains(&a.to_string())); + assert!(second_title.contains(&b.to_string())); + let one = p + .add_element(Kind::ExchangeItem, Layer::Oa, "Same name", None) + .unwrap(); + let two = p + .add_element(Kind::ExchangeItem, Layer::Oa, "Same name", None) + .unwrap(); + let field = p + .add_element(Kind::DataField, Layer::Oa, "Value", Some(one)) + .unwrap(); + let annotation = |path| { + FacetSelector::Annotation(KeyPair { + oa: Some(AnnotationKey { path, field }), + sa: None, + }) + }; + let first_title = super::view::title(&p, &annotation(vec![one, a])); + let second_title = super::view::title(&p, &annotation(vec![two, b])); + assert_ne!(first_title, second_title); + for id in [one, field] { + assert!(first_title.contains(&format!("@{}", p.elements[&id].alias))); + } + assert!(first_title.contains(&a.to_string())); + assert!(second_title.contains(&b.to_string())); + assert_ne!(super::view::title(&p, &annotation(vec![])), first_title); +} fn decide(app: &mut App, selected: FacetSelector, decision: SemanticDecision) { send(app, Event::Select(selected.clone())); send(app, Event::Decide(selected, Some(decision))); diff --git a/crates/oa-desktop/src/semantic_review/view.rs b/crates/oa-desktop/src/semantic_review/view.rs index f8443fa4..56468676 100644 --- a/crates/oa-desktop/src/semantic_review/view.rs +++ b/crates/oa-desktop/src/semantic_review/view.rs @@ -74,8 +74,12 @@ pub(super) fn title(project: &Project, facet: &FacetSelector) -> String { let keys = |keys: &KeyPair| { format!( "{} ↔ {}", - keys.oa.map(|id| project.named(id)).unwrap_or("Absent"), - keys.sa.map(|id| project.named(id)).unwrap_or("Absent") + keys.oa + .map(|id| choice(project, id).label) + .unwrap_or_else(|| "Absent".into()), + keys.sa + .map(|id| choice(project, id).label) + .unwrap_or_else(|| "Absent".into()) ) }; match facet { @@ -93,12 +97,12 @@ pub(super) fn title(project: &Project, facet: &FacetSelector) -> String { "Message value · {} ↔ {}", pair.oa .as_ref() - .map(|key| project.named(key.field)) - .unwrap_or("Absent"), + .map(|key| annotation_title(project, key)) + .unwrap_or_else(|| "Absent".into()), pair.sa .as_ref() - .map(|key| project.named(key.field)) - .unwrap_or("Absent") + .map(|key| annotation_title(project, key)) + .unwrap_or_else(|| "Absent".into()) ), FacetSelector::Binding(pair) => format!("Lifeline binding · {}", keys(pair)), FacetSelector::Child(pair) => format!("Owned child · {}", keys(pair)), @@ -118,6 +122,14 @@ pub(super) fn title(project: &Project, facet: &FacetSelector) -> String { } } } +fn annotation_title(project: &Project, key: &AnnotationKey) -> String { + key.path + .iter() + .chain(std::iter::once(&key.field)) + .map(|id| choice(project, *id).label) + .collect::>() + .join(" / ") +} fn drift(drift: Drift) -> &'static str { match drift { Drift::Unreviewed => "Unreviewed", diff --git a/harness/release-semantic-gui.md b/harness/release-semantic-gui.md index 3ff72baf..58f8ce88 100644 --- a/harness/release-semantic-gui.md +++ b/harness/release-semantic-gui.md @@ -42,7 +42,8 @@ Base: `d130013`. Dedicated build target: ## Verification -Verified against core `6bb2583`, using only the dedicated desktop target: +Verified after the independent scalar review fixes on SEM `6ad3cb`, using only the +dedicated desktop target (core behavior remains based on `6bb2583`): ```powershell $env:CARGO_TARGET_DIR='D:/CodexBuilds/Capella2-semantic-desktop-target' @@ -51,7 +52,8 @@ cargo clippy --locked -p oa-desktop --all-targets -- -D warnings cargo build --locked -p oa-desktop ``` -All **272 desktop tests passed**, including **11 new semantic review tests**. +All **277 desktop tests passed**, including **11 original semantic review tests** +and **5 scalar review regression tests**. Strict Clippy and the desktop build passed. The existing migration test's header assertion now follows `SCHEMA_VERSION` instead of expecting schema 4. @@ -64,6 +66,25 @@ relationship role/bound candidates with retained identity/endpoints; and exact inherited-annotation cleanup approval, cleared when decisions change. Unselected invalid scalar drafts remain retained without contaminating another candidate. +The independent scalar review found three gaps, now corrected: + +- Structural scalar edits replace only their authorized subtree. Clearing an + invalid selected bound, changing a variant, and adding/removing/reordering a + literal retain untouched raw inputs and editor buffers, including invalid + inputs belonging to another decision. +- Dependent default, reference and port controls read a noncommitting context + composed by the same scalar Keep/Take/Candidate assembler as the request. Raw + candidates remain separate. Incomplete inputs use their stored shape only for + those controls; request creation and Apply still reject invalid selected input. + The Boolean-source/Integer-target default reproduction now offers Boolean + literals in both directions. Tests also cover Candidate type, Keep reversion, + storage-dependent default controls and mechanism-dependent reference choices. +- Keyed fact headings display live names with stable aliases or deleted UUIDs. + Message annotation headings include the complete qualified path and field. + +The regression tests exercise App events, the actual default-form helpers and +button availability, exact source retention before Apply, and one-step undo. + Built executable: `D:/CodexBuilds/Capella2-semantic-desktop-target/debug/arcadia-studio.exe`. The build target is idle. No native application was launched by this task.