diff --git a/crates/oa-desktop/src/forms.rs b/crates/oa-desktop/src/forms.rs index 53695aef..9c2f1c76 100644 --- a/crates/oa-desktop/src/forms.rs +++ b/crates/oa-desktop/src/forms.rs @@ -46,6 +46,7 @@ pub struct Draft { pub inputs: BTreeMap, pub exact_strings: bool, context: Option, + message_timer_draft: Option, port_contract_drafts: BTreeMap, pub value_browsers: BTreeMap, } @@ -85,6 +86,7 @@ impl Draft { inputs: BTreeMap::new(), exact_strings: false, context: None, + message_timer_draft: None, port_contract_drafts: BTreeMap::new(), value_browsers: BTreeMap::new(), }; @@ -248,6 +250,29 @@ impl Draft { self.rebuild_retaining(|old| (!within(old, path)).then(|| old.to_owned())); Ok(()) } + /// Message-kind selection preserves the timer's exact input/editor state + /// while its payload is temporarily hidden by another kind. + pub fn message_kind_variant(&mut self, path: &str, value: Value) -> Result<(), String> { + serde_json::from_value::(value.clone()).map_err(|e| e.to_string())?; + let current = self + .value + .pointer(path) + .ok_or("Message kind no longer exists.")?; + if same_variant(current, &value) { + return Ok(()); + } + let timer_path = format!("{path}/Timer/expression"); + if let Some(input) = self.inputs.remove(&timer_path) { + self.message_timer_draft = Some(input); + } + self.shape_scoped(path, value)?; + if self.inputs.contains_key(&timer_path) + && let Some(input) = self.message_timer_draft.take() + { + self.inputs.insert(timer_path, input); + } + Ok(()) + } fn rebuild_retaining(&mut self, new_path: impl Fn(&str) -> Option) { let previous = std::mem::take(&mut self.inputs); self.rebuild(); diff --git a/crates/oa-desktop/src/semantic_review.rs b/crates/oa-desktop/src/semantic_review.rs index 229cbe10..f59ca4cb 100644 --- a/crates/oa-desktop/src/semantic_review.rs +++ b/crates/oa-desktop/src/semantic_review.rs @@ -8,10 +8,13 @@ use oa_core::{transitions::*, *}; use serde_json::Value; use std::collections::{BTreeMap, BTreeSet}; +mod message_kind; mod scalar; mod view; pub(crate) use view::summary; #[cfg(test)] +mod message_kind_tests; +#[cfg(test)] mod tests; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -54,6 +57,9 @@ pub struct State { pub decisions: BTreeMap, pub references: BTreeMap, pub draft: Option, + // Group editing cannot reshape the independently authored TimerExpression + // scalar draft. Both drafts survive decision changes until this trace closes. + pub message_draft: Option, pub plan: Option, pub proposed: Option, pub error: Option, @@ -96,6 +102,7 @@ impl State { decisions: BTreeMap::new(), references: BTreeMap::new(), draft: None, + message_draft: None, plan: None, proposed: None, error: None, @@ -128,6 +135,12 @@ impl State { self.draft = self .target(project) .and_then(|id| Draft::from_project(project, id)); + self.message_draft = self + .trace + .and_then(|id| project.traces.get(&id)) + .filter(|trace| message_kind::available(project, trace)) + .and_then(|_| self.target(project)) + .and_then(|id| Draft::from_project(project, id)); if let Some(trace) = self.trace.and_then(|id| project.traces.get(&id)) { self.references.insert(trace.oa, trace.sa); } @@ -188,7 +201,10 @@ impl State { self.edit(project, &path, |d| d.shape_scoped(&path, value)) } Event::Variant(path, value) => self.edit(project, &path, |d| { - if d.value + if path.ends_with("/message_kind") { + d.message_kind_variant(&path, value) + } else if d + .value .pointer(&path) .is_some_and(|v| crate::forms::same_variant(v, &value)) { @@ -286,9 +302,15 @@ impl State { .and_then(|id| scalar::binding(project, id, facet)) .ok_or("Selected scalar property is unavailable.")?; if path != root && !path.starts_with(&format!("{root}/")) { - return Err("Only the selected scalar property can change in this draft.".into()); + return Err("Only the selected review property can change in this draft.".into()); } - let draft = self.draft.as_mut().ok_or("Choose an existing target.")?; + let draft = if message_kind::handles(facet) { + &mut self.message_draft + } else { + &mut self.draft + } + .as_mut() + .ok_or("Choose an existing target.")?; // The scoped view sends only descendants of the selected scalar property. // Protected identity and top-level body/perspective controls are never exposed. if path == "/id" || path == "/alias" || path == "/body" || path == "/layer" { @@ -301,7 +323,21 @@ impl State { self.target(project) .and_then(|id| scalar::binding(project, id, facet)) .is_some() - && !scalar::grouped(facet) + && (!scalar::grouped(facet) + || (message_kind::handles(facet) + && self.message_draft.is_some() + && self + .trace + .and_then(|id| project.traces.get(&id)) + .is_some_and(|trace| message_kind::available(project, trace)))) + } + + fn candidate_draft(&self, facet: &FacetSelector) -> Option<&Draft> { + if message_kind::handles(facet) { + self.message_draft.as_ref() + } else { + self.draft.as_ref() + } } pub fn can_acknowledge_constructor(&self, project: &Project, facet: &FacetSelector) -> bool { @@ -319,13 +355,12 @@ impl State { { return false; } - self.facts - .iter() - .find(|f| f.selector == *facet) - .is_some_and(|f| { - scalar::value(&f.current, Layer::Oa).is_some() - && scalar::observation(&f.current, Layer::Oa) - == scalar::observation(&f.current, Layer::Sa) + capture_facet(project, trace.oa, trace.sa, facet) + .ok() + .is_some_and(|current| { + scalar::value(¤t, Layer::Oa).is_some() + && scalar::observation(¤t, Layer::Oa) + == scalar::observation(¤t, Layer::Sa) }) } @@ -399,7 +434,6 @@ impl State { 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 { @@ -412,6 +446,7 @@ impl State { } } map.insert(source, target); + let mut message_candidate = None; for (facet, decision) in &self.decisions { if *decision == SemanticDecision::KeepTarget { continue; @@ -421,6 +456,9 @@ impl State { let value = match decision { SemanticDecision::KeepTarget => continue, SemanticDecision::Candidate => { + let draft = self + .candidate_draft(facet) + .ok_or("Target draft is unavailable.")?; let value = match draft.materialize_scopes(std::slice::from_ref(&path)) { Ok(value) => value, Err(_) if form_context => draft.value.clone(), @@ -431,15 +469,39 @@ impl State { .cloned() .ok_or("Candidate property is no longer available.")? } + SemanticDecision::TakeSource if message_kind::handles(facet) => { + // First equal kinds acknowledge only the discriminator. + // An unselected Timer payload must stay at the destination. + message_kind::source_value( + project, + if self.can_acknowledge_constructor(project, facet) { + target + } else { + source + }, + )? + } SemanticDecision::TakeSource => { let fact = capture_facet(project, trace.oa, trace.sa, facet)?; scalar::source_value(&fact, self.direction.source_layer(), &map)? } }; + if message_kind::handles(facet) && *decision == SemanticDecision::Candidate { + message_candidate = Some((path.clone(), value.clone())); + } *final_value .pointer_mut(&path) .ok_or("Final target property is unavailable.")? = value; } + if !form_context + && let Some((path, candidate)) = message_candidate + && let Ok(current) = message_kind::source_value(project, target) + && crate::forms::same_variant(&candidate, ¤t) + && candidate != current + && final_value.pointer(&path) != Some(&candidate) + { + return Err("The message-kind candidate and Timer Expression decision propose different timer values. Keep the kind or make their final values agree; both drafts are retained.".into()); + } Ok(final_value) } @@ -449,6 +511,9 @@ impl State { self.error = None; let context = self.combined_value(project, true).ok(); if let Some(draft) = &mut self.draft { + draft.set_context(context.clone()); + } + if let Some(draft) = &mut self.message_draft { draft.set_context(context); } let Some(trace) = self.trace.and_then(|id| project.traces.get(&id)) else { diff --git a/crates/oa-desktop/src/semantic_review/message_kind.rs b/crates/oa-desktop/src/semantic_review/message_kind.rs new file mode 100644 index 00000000..4d11dedc --- /dev/null +++ b/crates/oa-desktop/src/semantic_review/message_kind.rs @@ -0,0 +1,32 @@ +//! The first supported constructor group is the closed nested MessageKind. +//! The core owns its acknowledgment/retirement and validation policies. +use super::*; + +pub(super) fn selector() -> FacetSelector { + FacetSelector::Occurrence(OccurrenceFacet::MessageKind) +} +pub(super) fn timer_selector() -> FacetSelector { + FacetSelector::Occurrence(OccurrenceFacet::TimerExpression) +} +pub(super) fn handles(facet: &FacetSelector) -> bool { + *facet == selector() +} +pub(super) fn value(project: &Project, id: Id) -> Option<&MessageKind> { + match project.elements.get(&id)?.shared()? { + SharedBody::Occurrence { + body: OccurrenceBody::Exchange { message_kind, .. }, + .. + } => Some(message_kind), + _ => None, + } +} +pub(super) fn available(project: &Project, trace: &Trace) -> bool { + value(project, trace.oa).is_some() && value(project, trace.sa).is_some() +} +/// MessageConstructor observations contain only the discriminator. Taking a +/// replacing group needs the complete typed payload, including Timer.expression. +pub(super) fn source_value(project: &Project, id: Id) -> Result { + let value = + value(project, id).ok_or("Message kind requires an existing Exchange occurrence.")?; + serde_json::to_value(value).map_err(|e| e.to_string()) +} diff --git a/crates/oa-desktop/src/semantic_review/message_kind_tests.rs b/crates/oa-desktop/src/semantic_review/message_kind_tests.rs new file mode 100644 index 00000000..82f6b8bc --- /dev/null +++ b/crates/oa-desktop/src/semantic_review/message_kind_tests.rs @@ -0,0 +1,691 @@ +use super::{tests::*, *}; +use crate::{app::App, document::Document}; +use serde_json::json; + +fn fixture(oa_kind: MessageKind, sa_kind: MessageKind) -> (Project, Id, Id, Id) { + let mut p = Project::empty(); + let mut ids = vec![]; + for (layer, message_kind) in [(Layer::Oa, oa_kind), (Layer::Sa, sa_kind)] { + let scenario = p + .add_element(Kind::Scenario, layer, "Scenario", None) + .unwrap(); + let id = p + .add_element(Kind::Occurrence, layer, "Message", Some(scenario)) + .unwrap(); + *p.elements.get_mut(&id).unwrap().shared_mut().unwrap() = SharedBody::Occurrence { + body: OccurrenceBody::Exchange { + exchange: None, + sender: None, + receiver: None, + message_kind, + reply_to: None, + values: vec![], + }, + precondition: "retain precondition".into(), + postcondition: "retain postcondition".into(), + }; + p.elements + .get_mut(&scenario) + .unwrap() + .steps_mut() + .unwrap() + .push(id); + ids.push(id); + } + let trace = add_trace(&mut p, ids[0], ids[1], "Message-kind review"); + (p, ids[0], ids[1], trace) +} +fn timer(value: &str) -> MessageKind { + MessageKind::Timer { + expression: value.into(), + } +} +fn path(layer: &str) -> String { + format!("/body/{layer}/Shared/Occurrence/body/Exchange/message_kind") +} +fn set_kind(p: &mut Project, id: Id, kind: MessageKind) { + let Some(SharedBody::Occurrence { + body: OccurrenceBody::Exchange { message_kind, .. }, + .. + }) = p.elements.get_mut(&id).unwrap().shared_mut() + else { + panic!() + }; + *message_kind = kind; +} +fn seed_history(p: &mut Project, trace: Id) { + let t = &p.traces[&trace]; + let baseline = TraceBaseline { + oa: p.properties(t.oa).unwrap(), + sa: p.properties(t.sa).unwrap(), + semantic: Some(SemanticBaseline { + facets: [message_kind::selector(), message_kind::timer_selector()] + .iter() + .map(|selector| capture_facet(p, t.oa, t.sa, selector).unwrap()) + .collect(), + }), + }; + p.traces.get_mut(&trace).unwrap().baseline = Some(baseline); +} +fn acknowledged(app: &App) -> BTreeSet { + state(app).plan.as_ref().unwrap().reviews[0] + .acknowledged + .iter() + .cloned() + .collect() +} +fn failures(app: &App) -> String { + state(app) + .error + .iter() + .chain(state(app).plan.iter().flat_map(|p| &p.conflicts)) + .cloned() + .collect::>() + .join("; ") +} + +#[test] +fn message_group_take_full_timer_and_scalar_candidate_are_one_source_history_change_both_directions() + { + for reverse in [false, true] { + let expression = " after β ≥ 5 s\n\tkeep exact whitespace "; + let (p, oa, sa, trace) = if reverse { + fixture(MessageKind::Asynchronous, timer(expression)) + } else { + fixture(timer(expression), MessageKind::Asynchronous) + }; + let (source, target) = if reverse { (sa, oa) } else { (oa, sa) }; + let mut app = App::with_project(p); + let mut bundle = app.document.bundle().clone(); + bundle + .files + .get_mut(&bundle.entry) + .unwrap() + .insert_str(0, "// Keep the engineering review rationale.\r\n"); + app.document.apply_bundle(bundle).unwrap(); + let before = app.document.snapshot(); + let revision = app.document.revision(); + start(&mut app, source); + decide( + &mut app, + message_kind::selector(), + SemanticDecision::TakeSource, + ); + decide( + &mut app, + FacetSelector::Header(HeaderFacet::Name), + SemanticDecision::Candidate, + ); + send( + &mut app, + Event::Input("/name".into(), "Reviewed message".into()), + ); + assert!(state(&app).can_apply(), "{:?}", state(&app).error); + assert_eq!( + acknowledged(&app), + BTreeSet::from([ + message_kind::selector(), + message_kind::timer_selector(), + FacetSelector::Header(HeaderFacet::Name) + ]) + ); + assert!( + state(&app).plan.as_ref().unwrap().reviews[0] + .retired + .is_empty() + ); + assert!(state(&app).plan.as_ref().unwrap().impacts.is_empty()); + let expected = state(&app).proposed.clone().unwrap(); + assert_eq!( + message_kind::value(&expected, target), + Some(&timer(expression)) + ); + assert_eq!(app.document.bundle(), &before.bundle); + apply(&mut app); + assert_eq!(app.project(), &expected); + assert_eq!(app.document.revision(), revision + 1); + assert_eq!( + app.project().elements[&source], + before.decoded.project.elements[&source] + ); + let saved = app.document.bundle().clone(); + assert!(saved.files[&saved.entry].starts_with("// Keep the engineering review rationale.")); + assert_eq!( + Document::from_bundle(saved.clone(), source_project::ProjectLimits::default()) + .unwrap() + .snapshot() + .decoded + .project, + expected + ); + let facts = &app.project().traces[&trace] + .baseline + .as_ref() + .unwrap() + .semantic + .as_ref() + .unwrap() + .facets; + assert_eq!(facts.len(), 3); + let _ = app.update(Message::Undo); + assert_eq!(app.document.bundle(), &before.bundle); + let _ = app.update(Message::Redo); + assert_eq!(app.document.bundle(), &saved); + } +} + +#[test] +fn message_group_candidate_timer_switches_preserve_both_drafts_and_overlap_is_explicit() { + let (p, oa, _, _) = fixture(MessageKind::Asynchronous, timer("original target")); + let mut app = App::with_project(p); + let before = app.document.snapshot(); + let kind = path("Sa"); + let expression = format!("{kind}/Timer/expression"); + start(&mut app, oa); + decide( + &mut app, + message_kind::timer_selector(), + SemanticDecision::Candidate, + ); + send( + &mut app, + Event::Input(expression.clone(), "independent scalar timer".into()), + ); + decide( + &mut app, + message_kind::selector(), + SemanticDecision::Candidate, + ); + assert_eq!(state(&app).decisions.len(), 2); + assert!(!state(&app).can_apply()); + send( + &mut app, + Event::Input(expression.clone(), " group β timer\n\t ".into()), + ); + send( + &mut app, + Event::Text(expression.clone(), text_editor::Action::SelectAll), + ); + let cursor = state(&app).message_draft.as_ref().unwrap().inputs[&expression] + .editor + .cursor(); + let selection = state(&app).message_draft.as_ref().unwrap().inputs[&expression] + .editor + .selection(); + send( + &mut app, + Event::Variant(kind.clone(), json!("Asynchronous")), + ); + send( + &mut app, + Event::Variant(kind, json!({"Timer":{"expression":""}})), + ); + assert_eq!( + state(&app).message_draft.as_ref().unwrap().inputs[&expression].raw, + " group β timer\n\t " + ); + assert_eq!( + state(&app).message_draft.as_ref().unwrap().inputs[&expression] + .editor + .cursor(), + cursor + ); + assert_eq!( + state(&app).message_draft.as_ref().unwrap().inputs[&expression] + .editor + .selection(), + selection + ); + assert_eq!( + state(&app).draft.as_ref().unwrap().inputs[&expression].raw, + "independent scalar timer" + ); + assert!(!state(&app).can_apply()); + assert_eq!(app.document.bundle(), &before.bundle); + // The visible overlap-resolution actions only change the explicitly chosen + // decision. Neither independent candidate buffer is discarded. + send(&mut app, Event::Decide(message_kind::selector(), None)); + assert!(state(&app).can_apply()); + send( + &mut app, + Event::Decide(message_kind::timer_selector(), None), + ); + decide( + &mut app, + message_kind::selector(), + SemanticDecision::Candidate, + ); + assert!(state(&app).can_apply(), "{:?}", state(&app).error); + assert_eq!( + acknowledged(&app), + BTreeSet::from([message_kind::selector(), message_kind::timer_selector()]) + ); + let _ = state(&app).view(app.project()); + let expected = state(&app).proposed.clone().unwrap(); + apply(&mut app); + assert_eq!(app.project(), &expected); +} + +#[test] +fn message_group_keep_historical_switch_retires_timer_and_completed_no_switch_rejects() { + let (mut p, oa, sa, trace) = fixture(timer("old OA"), timer("old SA")); + seed_history(&mut p, trace); + set_kind(&mut p, oa, MessageKind::Asynchronous); + set_kind(&mut p, sa, MessageKind::Asynchronous); + let mut app = App::with_project(p); + start(&mut app, oa); + decide( + &mut app, + message_kind::selector(), + SemanticDecision::KeepTarget, + ); + assert!(state(&app).can_apply(), "{:?}", state(&app).error); + assert_eq!( + acknowledged(&app), + BTreeSet::from([message_kind::selector()]) + ); + assert_eq!( + state(&app).plan.as_ref().unwrap().reviews[0].retired, + vec![message_kind::timer_selector()] + ); + apply(&mut app); + assert_eq!( + app.project().traces[&trace] + .baseline + .as_ref() + .unwrap() + .semantic + .as_ref() + .unwrap() + .facets + .len(), + 1 + ); + start(&mut app, oa); + decide( + &mut app, + message_kind::selector(), + SemanticDecision::KeepTarget, + ); + assert!(!state(&app).can_apply()); + assert!( + failures(&app).contains("No constructor switch"), + "{}", + failures(&app) + ); +} + +#[test] +fn message_group_reply_and_reference_take_require_explicit_call_mapping_both_directions() { + for reverse in [false, true] { + let (mut p, oa, sa, _) = if reverse { + fixture(MessageKind::Asynchronous, MessageKind::Reply) + } else { + fixture(MessageKind::Reply, MessageKind::Asynchronous) + }; + let (source, target, layer) = if reverse { + (sa, oa, "Oa") + } else { + (oa, sa, "Sa") + }; + let mut calls = vec![]; + for message in [oa, sa] { + let scenario = p.elements[&message].parent.unwrap(); + let layer = p.elements[&message].layer(); + let call = p + .add_element(Kind::Occurrence, layer, "Correlated call", Some(scenario)) + .unwrap(); + *p.elements.get_mut(&call).unwrap().shared_mut().unwrap() = SharedBody::Occurrence { + body: OccurrenceBody::Exchange { + exchange: None, + sender: None, + receiver: None, + message_kind: MessageKind::Call, + reply_to: None, + values: vec![], + }, + precondition: String::new(), + postcondition: String::new(), + }; + p.elements + .get_mut(&scenario) + .unwrap() + .steps_mut() + .unwrap() + .insert(0, call); + if message == source { + let Some(SharedBody::Occurrence { + body: OccurrenceBody::Exchange { reply_to, .. }, + .. + }) = p.elements.get_mut(&message).unwrap().shared_mut() + else { + panic!() + }; + *reply_to = Some(call); + } + calls.push(call); + } + let target_call = if reverse { calls[0] } else { calls[1] }; + let mut app = App::with_project(p); + let before = app.document.snapshot(); + start(&mut app, source); + decide( + &mut app, + message_kind::selector(), + SemanticDecision::Candidate, + ); + send(&mut app, Event::Variant(path(layer), json!("Reply"))); + decide( + &mut app, + FacetSelector::Occurrence(OccurrenceFacet::ReplyTo), + SemanticDecision::TakeSource, + ); + assert!(!state(&app).can_apply()); + assert!( + failures(&app).contains("reference counterparts"), + "{}", + failures(&app) + ); + send( + &mut app, + Event::ReferenceOa(Choice { + id: calls[0], + label: "OA call".into(), + }), + ); + send( + &mut app, + Event::ReferenceSa(Choice { + id: calls[1], + label: "SA call".into(), + }), + ); + send(&mut app, Event::AddReference); + assert!(state(&app).can_apply(), "{}", failures(&app)); + assert_eq!( + acknowledged(&app), + BTreeSet::from([ + message_kind::selector(), + FacetSelector::Occurrence(OccurrenceFacet::ReplyTo) + ]) + ); + let final_value = + serde_json::to_value(&state(&app).proposed.as_ref().unwrap().elements[&target]) + .unwrap(); + assert_eq!( + final_value.pointer(&format!( + "/body/{layer}/Shared/Occurrence/body/Exchange/reply_to" + )), + Some(&json!(target_call)) + ); + assert_eq!(app.document.bundle(), &before.bundle); + apply(&mut app); + assert_eq!( + message_kind::value(app.project(), target), + Some(&MessageKind::Reply) + ); + for call in calls { + assert_eq!( + app.project().elements[&call], + before.decoded.project.elements[&call] + ); + } + } +} + +#[test] +fn message_group_candidate_closed_kinds_are_core_validated_in_both_directions() { + for reverse in [false, true] { + for kind in [ + MessageKind::Asynchronous, + MessageKind::Call, + MessageKind::Reply, + MessageKind::Create, + MessageKind::Delete, + timer("after 2 s"), + ] { + let (p, oa, sa, _) = fixture(MessageKind::Asynchronous, MessageKind::Asynchronous); + let (source, target, layer) = if reverse { + (sa, oa, "Oa") + } else { + (oa, sa, "Sa") + }; + let mut app = App::with_project(p); + start(&mut app, source); + decide( + &mut app, + message_kind::selector(), + SemanticDecision::Candidate, + ); + send( + &mut app, + Event::Variant(path(layer), serde_json::to_value(&kind).unwrap()), + ); + assert!(state(&app).can_apply(), "{kind:?}: {}", failures(&app)); + assert_eq!( + message_kind::value(state(&app).proposed.as_ref().unwrap(), target), + Some(&kind) + ); + assert_eq!( + acknowledged(&app).contains(&message_kind::timer_selector()), + matches!(kind, MessageKind::Timer { .. }) + ); + assert!(state(&app).plan.as_ref().unwrap().impacts.is_empty()); + } + } + let (mut p, oa, _, _) = fixture(MessageKind::Asynchronous, MessageKind::Asynchronous); + let mut review = State::new(&p, oa).unwrap(); + let Some(SharedBody::Occurrence { body, .. }) = p.elements.get_mut(&oa).unwrap().shared_mut() + else { + panic!() + }; + *body = OccurrenceBody::Action { + action: None, + lifeline: None, + }; + review.refresh(&p); + assert!( + !review.editable(&p, &message_kind::selector()), + "An externally changed outer occurrence requires its own constructor review" + ); + review.update( + &p, + Event::Decide(message_kind::selector(), Some(SemanticDecision::Candidate)), + ); + assert!(review.decisions.is_empty()); + assert!(!review.can_apply()); +} + +#[test] +fn message_group_timer_does_not_erase_exchange_without_its_own_selected_decision() { + let (mut p, oa, sa, _) = fixture(MessageKind::Asynchronous, MessageKind::Asynchronous); + let sender = p + .add_element(Kind::Function, Layer::Sa, "Sender", None) + .unwrap(); + let receiver = p + .add_element(Kind::Function, Layer::Sa, "Receiver", None) + .unwrap(); + let exchange = p.relate( + RelationKind::FunctionalExchange, + "Message exchange", + sender, + receiver, + ); + let Some(SharedBody::Occurrence { + body: OccurrenceBody::Exchange { + exchange: selected, .. + }, + .. + }) = p.elements.get_mut(&sa).unwrap().shared_mut() + else { + panic!() + }; + *selected = Some(exchange); + let mut app = App::with_project(p); + let before = app.document.snapshot(); + start(&mut app, oa); + decide( + &mut app, + message_kind::selector(), + SemanticDecision::Candidate, + ); + send( + &mut app, + Event::Variant(path("Sa"), json!({"Timer":{"expression":"after 2 s"}})), + ); + assert!(!state(&app).can_apply()); + assert!( + failures(&app).contains("timer_contract"), + "{}", + failures(&app) + ); + assert_eq!(state(&app).decisions.len(), 1); + assert_eq!(app.document.bundle(), &before.bundle); + decide( + &mut app, + FacetSelector::Occurrence(OccurrenceFacet::Exchange), + SemanticDecision::Candidate, + ); + send( + &mut app, + Event::Shape( + "/body/Sa/Shared/Occurrence/body/Exchange/exchange".into(), + Value::Null, + ), + ); + assert!(state(&app).can_apply(), "{}", failures(&app)); + assert_eq!( + acknowledged(&app), + BTreeSet::from([ + message_kind::selector(), + message_kind::timer_selector(), + FacetSelector::Occurrence(OccurrenceFacet::Exchange) + ]) + ); + assert!(state(&app).plan.as_ref().unwrap().impacts.is_empty()); + apply(&mut app); + assert_eq!( + app.project().relations[&exchange], + before.decoded.project.relations[&exchange] + ); +} + +#[test] +fn message_group_initial_equal_take_acknowledges_only_kind_and_checks_authored_timer_agreement() { + let (p, oa, sa, _) = fixture(timer("source timer"), timer("target timer")); + let mut app = App::with_project(p); + start(&mut app, oa); + decide( + &mut app, + message_kind::selector(), + SemanticDecision::TakeSource, + ); + decide( + &mut app, + FacetSelector::Header(HeaderFacet::Name), + SemanticDecision::Candidate, + ); + send(&mut app, Event::Input("/name".into(), "Final name".into())); + assert!(state(&app).can_apply(), "{:?}", state(&app).error); + assert_eq!( + message_kind::value(state(&app).proposed.as_ref().unwrap(), sa), + Some(&timer("target timer")) + ); + assert!(!acknowledged(&app).contains(&message_kind::timer_selector())); + let expression = format!("{}/Timer/expression", path("Sa")); + decide( + &mut app, + message_kind::selector(), + SemanticDecision::Candidate, + ); + send( + &mut app, + Event::Input(expression.clone(), "authored group timer".into()), + ); + assert!( + !state(&app).can_apply(), + "An unchanged initial kind cannot silently authorize a timer edit" + ); + decide( + &mut app, + message_kind::timer_selector(), + SemanticDecision::Candidate, + ); + send( + &mut app, + Event::Input(expression.clone(), "different scalar timer".into()), + ); + assert!(!state(&app).can_apply()); + assert!( + state(&app) + .error + .as_ref() + .unwrap() + .contains("different timer values") + ); + send( + &mut app, + Event::Input(expression, "authored group timer".into()), + ); + assert!(state(&app).can_apply(), "{:?}", state(&app).error); + assert!(acknowledged(&app).contains(&message_kind::timer_selector())); + assert_eq!( + message_kind::value(state(&app).proposed.as_ref().unwrap(), sa), + Some(&timer("authored group timer")) + ); +} + +#[test] +fn message_group_rejects_protected_edits_stale_plan_and_source_failure_without_losing_timer() { + for source_failure in [false, true] { + let (p, oa, _, _) = fixture(MessageKind::Asynchronous, timer("target")); + let mut app = App::with_project(p); + if source_failure { + let bundle = app.document.bundle().clone(); + let mut limits = source_project::ProjectLimits::default(); + limits.model.max_source_bytes = + bundle.files.values().map(String::len).sum::() + 10; + app.document = Document::from_bundle(bundle, limits).unwrap(); + } + start(&mut app, oa); + decide( + &mut app, + message_kind::selector(), + SemanticDecision::Candidate, + ); + let expression = format!("{}/Timer/expression", path("Sa")); + let raw = "retain exact timer β\n".repeat(40); + send(&mut app, Event::Input(expression.clone(), raw.clone())); + assert!(state(&app).can_apply(), "{:?}", state(&app).error); + send( + &mut app, + Event::Input("/name".into(), "Unselected protected scope".into()), + ); + assert!(!state(&app).can_apply()); + assert!( + state(&app) + .error + .as_ref() + .unwrap() + .contains("selected review") + ); + send(&mut app, Event::Preview); + assert!(state(&app).can_apply()); + if !source_failure { + app.document + .execute(Command::Rename(oa, "External source edit".into())) + .unwrap(); + } + let before = app.document.snapshot(); + let revision = app.document.revision(); + apply(&mut app); + assert!(state(&app).error.is_some()); + assert_eq!(state(&app).decisions.len(), 1); + assert_eq!( + state(&app).message_draft.as_ref().unwrap().inputs[&expression].raw, + raw + ); + assert_eq!(app.document.bundle(), &before.bundle); + assert_eq!(app.document.revision(), revision); + } +} diff --git a/crates/oa-desktop/src/semantic_review/tests.rs b/crates/oa-desktop/src/semantic_review/tests.rs index 788f8eb3..9b775c70 100644 --- a/crates/oa-desktop/src/semantic_review/tests.rs +++ b/crates/oa-desktop/src/semantic_review/tests.rs @@ -2,13 +2,13 @@ use super::*; use crate::{app::App, document::Document, workflow::Workflow}; use serde_json::json; -fn send(app: &mut App, event: Event) { +pub(super) fn send(app: &mut App, event: Event) { let _ = app.update(Message::Workflow(crate::workflow::Event::Semantic(event))); } -fn apply(app: &mut App) { +pub(super) fn apply(app: &mut App) { let _ = app.update(Message::Workflow(crate::workflow::Event::Apply)); } -fn state(app: &App) -> &State { +pub(super) fn state(app: &App) -> &State { let Some(Workflow::Semantic(state)) = &app.workflow else { panic!("Semantic workflow missing: {}", app.status) }; @@ -35,7 +35,7 @@ fn fixture() -> (Project, Id, Id, Id) { let trace = add_trace(&mut p, oa, sa, "Range rationale"); (p, oa, sa, trace) } -fn add_trace(p: &mut Project, oa: Id, sa: Id, name: &str) -> Id { +pub(super) fn add_trace(p: &mut Project, oa: Id, sa: Id, name: &str) -> Id { let id = Id::new_v4(); p.traces.insert( id, @@ -52,7 +52,7 @@ fn add_trace(p: &mut Project, oa: Id, sa: Id, name: &str) -> Id { ); id } -fn start(app: &mut App, source: Id) { +pub(super) fn start(app: &mut App, source: Id) { let _ = app.update(Message::StartSemanticReview(source)); } fn facet(value: DataFacet) -> FacetSelector { @@ -262,7 +262,7 @@ fn independent_review_keyed_fact_titles_keep_live_deleted_and_qualified_identity 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) { +pub(super) fn decide(app: &mut App, selected: FacetSelector, decision: SemanticDecision) { send(app, Event::Select(selected.clone())); send(app, Event::Decide(selected, Some(decision))); } @@ -386,7 +386,7 @@ fn invalid_numeric_draft_and_protected_edits_retain_workflow_and_source() { .error .as_ref() .unwrap() - .contains("selected scalar") + .contains("selected review") ); assert_eq!(app.project(), &before.decoded.project); send(&mut app, Event::Preview); diff --git a/crates/oa-desktop/src/semantic_review/view.rs b/crates/oa-desktop/src/semantic_review/view.rs index 56468676..f55ffad8 100644 --- a/crates/oa-desktop/src/semantic_review/view.rs +++ b/crates/oa-desktop/src/semantic_review/view.rs @@ -266,7 +266,7 @@ impl State { .iter() .filter(|f| f.drift == Drift::Unreviewed) .count(); - content = content.push(text(format!("{} selected decisions · {unreviewed} currently unreviewed facts · Constructors and structured collections require further review.", self.decisions.len())).size(10).color(style::MUTED)); + content = content.push(text(format!("{} selected decisions · {unreviewed} currently unreviewed facts · Other constructor groups and structured collections require further review.", self.decisions.len())).size(10).color(style::MUTED)); if let Some(trace) = trace { content = content.push(text(summary(project, trace)).size(10).color(style::MUTED)); } @@ -364,7 +364,32 @@ impl State { .and_then(|t| t.baseline.as_ref()) .and_then(|b| b.semantic.as_ref()) .and_then(|b| b.facets.iter().find(|f| f.selector() == fact.selector)); + let is_message_kind = message_kind::handles(&fact.selector); + let timer_previous = trace + .and_then(|t| t.baseline.as_ref()) + .and_then(|b| b.semantic.as_ref()) + .and_then(|b| { + b.facets + .iter() + .find(|f| f.selector() == message_kind::timer_selector()) + }); + let timer_current = self + .facts + .iter() + .find(|f| f.selector == message_kind::timer_selector()); let side = |layer, label: &str| { + let mut before = observed(project, previous, layer); + let mut current = observed(project, Some(&fact.current), layer); + if is_message_kind { + before.push_str(&format!( + "\nTimer expression: {}", + observed(project, timer_previous, layer) + )); + current.push_str(&format!( + "\nTimer expression: {}", + observed(project, timer_current.map(|f| &f.current), layer) + )); + } column![ text(format!( "{label}{}", @@ -377,9 +402,9 @@ impl State { .size(12) .color(style::TEAL), text("Previously reviewed").size(10).color(style::MUTED), - text(observed(project, previous, layer)).size(12), + text(before).size(12), text("Current").size(10).color(style::MUTED), - text(observed(project, Some(&fact.current), layer)).size(12), + text(current).size(12), ] .spacing(5) .width(Fill) @@ -401,11 +426,14 @@ impl State { .color(style::TEAL), ) .push( - text(observed( - proposed, - Some(&after), - self.direction.target_layer(), - )) + text(if is_message_kind { + self.target(project) + .and_then(|id| message_kind::source_value(proposed, id).ok()) + .map(|value| formatted(proposed, &value, &BTreeSet::new())) + .unwrap_or_else(|| "Message kind is not applicable.".into()) + } else { + observed(proposed, Some(&after), self.direction.target_layer()) + }) .size(12), ); } else { @@ -416,6 +444,12 @@ impl State { ); } if self.editable(project, &fact.selector) { + if is_message_kind { + detail = detail.push(text("A replacing message-kind decision reviews its Timer Expression as a group. The exact acknowledgment and retirement appear below.").size(12)); + if self.can_acknowledge_constructor(project, &fact.selector) { + detail = detail.push(text("These initial kinds are equal. Keep or Take acknowledges only the kind; an unchanged final kind also needs a separate Timer Expression decision for expression edits.").size(11).color(style::MUTED)); + } + } let destination = if self.direction.target_layer() == Layer::Sa { "System" } else { @@ -438,15 +472,19 @@ impl State { ] .spacing(5); buttons = buttons.push(action( - "Set final value…", + if is_message_kind { + "Set final message kind…" + } else { + "Set final value…" + }, Event::Decide(fact.selector.clone(), Some(SemanticDecision::Candidate)), )); - detail = detail.push(buttons).push(action( + detail = detail.push(buttons.wrap()).push(action( "Leave unselected", Event::Decide(fact.selector.clone(), None), )); if self.decisions.get(&fact.selector) == Some(&SemanticDecision::Candidate) - && let Some(draft) = &self.draft + && let Some(draft) = self.candidate_draft(&fact.selector) && let Some(path) = self .target(project) .and_then(|id| scalar::binding(project, id, &fact.selector)) @@ -463,6 +501,38 @@ impl State { } else { detail = detail.push(text(if scalar::grouped(&fact.selector) { "Constructor groups are shown for comparison. Group replacement and its complete descendant review are a later milestone." } else { "This structured or inapplicable fact is read-only here. Its review history is retained." }).size(12).color(style::MUTED)); } + if is_message_kind || fact.selector == message_kind::timer_selector() { + if self.decisions.contains_key(&message_kind::selector()) + && self.decisions.contains_key(&message_kind::timer_selector()) + { + detail = detail.push(text("Both kind and timer decisions are selected. An initial equal-kind acknowledgment can coexist with a timer decision; a replacing group requires one combined decision. Resolve any overlap explicitly; both drafts are retained.").size(11)) + .push(action("Replace timer decision with message-kind group", Event::Decide(message_kind::timer_selector(), None))) + .push(action("Leave message kind unselected; use timer decision", Event::Decide(message_kind::selector(), None))); + } + if let Some(review) = self.plan.as_ref().and_then(|p| p.reviews.first()) { + detail = detail.push(text("Exact review scope from preview").size(13)); + for (label, selectors) in [ + ("Acknowledge", &review.acknowledged), + ("Retire previous observation", &review.retired), + ] { + detail = detail.push( + text(format!( + "{label}: {}", + if selectors.is_empty() { + "None".into() + } else { + selectors + .iter() + .map(|s| title(project, s)) + .collect::>() + .join(", ") + } + )) + .size(11), + ); + } + } + } if matches!( fact.selector, FacetSelector::Child(_) diff --git a/crates/oa-desktop/src/views.rs b/crates/oa-desktop/src/views.rs index 88719043..7c427e43 100644 --- a/crates/oa-desktop/src/views.rs +++ b/crates/oa-desktop/src/views.rs @@ -744,6 +744,7 @@ fn traces(app: &App) -> View<'_, Message> { action("+ Artifact", Message::AddArtifact) ] .spacing(8) + .wrap() ] .spacing(16); for t in p.traces.values() { diff --git a/harness/release-semantic-gui.md b/harness/release-semantic-gui.md index 58f8ce88..7f7ff52e 100644 --- a/harness/release-semantic-gui.md +++ b/harness/release-semantic-gui.md @@ -1,7 +1,8 @@ -# Semantic desktop review — milestones 1 and 2 +# Semantic desktop review — scalars and nested message kind -Status: milestones 1–2 implemented and desktop tests/build verified; not full GUI -parity, native acceptance or release acceptance. +Status: milestones 1–2 implemented; the bounded MessageKind/TimerExpression +constructor slice is implemented. Full milestone 3, GUI parity, native acceptance +and release acceptance remain outstanding. Base: `d130013`. Dedicated build target: `D:/CodexBuilds/Capella2-semantic-desktop-target`. Native verification is root-owned. @@ -16,7 +17,8 @@ Base: `d130013`. Dedicated build target: - Support every applicable scalar leaf through Keep, Take and typed final-value forms. Candidate records contain the complete combined selected result. Identity, alias and layer are protected. The closed core remains the validation boundary. -- State.Kind is a single structural scalar. Other constructor switches, keyed +- State.Kind is a single structural scalar. Occurrence.MessageKind now supports + its complete nested group, including TimerExpression. Other constructor switches, keyed membership/order/annotation/binding/attachment decisions and pairing resets belong to milestone 3; show them read-only with an honest explanation. - Core `6bb2583` permits first equal constructor acknowledgment without descendant @@ -36,14 +38,16 @@ Base: `d130013`. Dedicated build target: 2. Scalar decisions and typed candidates; immutable initial observations; combined final records; exact impact acceptance; atomic document/source commit. Focused App event/source/history tests, strict Clippy and a dedicated desktop build. -3. Constructor closure, keyed and ordered facts, reset, complete artifact lifecycle, - multi-source review and remaining source/REST parity cases. Still outstanding. +3. Nested MessageKind/TimerExpression closure is implemented. Other constructor + groups, keyed and ordered facts, reset, complete artifact lifecycle, multi-source + review and remaining source/REST parity cases are outstanding. 4. Root-owned native review, cross-surface fixtures, package/release acceptance. ## Verification -Verified after the independent scalar review fixes on SEM `6ad3cb`, using only the -dedicated desktop target (core behavior remains based on `6bb2583`): +Scalar fixes were verified on SEM `6ad3cb`; the nested message-kind slice builds +on scalar commit `5a51a75`. Verification uses only the dedicated desktop target +(core behavior remains based on `6bb2583`): ```powershell $env:CARGO_TARGET_DIR='D:/CodexBuilds/Capella2-semantic-desktop-target' @@ -52,8 +56,8 @@ cargo clippy --locked -p oa-desktop --all-targets -- -D warnings cargo build --locked -p oa-desktop ``` -All **277 desktop tests passed**, including **11 original semantic review tests** -and **5 scalar review regression tests**. +All **285 desktop tests passed**, including **11 original semantic review tests**, +**5 scalar review regression tests**, and **8 nested message-kind tests**. Strict Clippy and the desktop build passed. The existing migration test's header assertion now follows `SCHEMA_VERSION` instead of expecting schema 4. @@ -85,19 +89,56 @@ The independent scalar review found three gaps, now corrected: The regression tests exercise App events, the actual default-form helpers and button availability, exact source retention before Apply, and one-step undo. +## Bounded constructor slice + +Occurrence.MessageKind is a complete closed nested family: Asynchronous, Call, +Reply, Create, Delete and Timer. It is available only when both traced counterparts +are Exchange occurrences. The only constructor-specific payload is the timer +expression. Outer occurrence replacement remains read-only. + +- Keep/Take/Candidate use the core's switch and first-equal acknowledgment policy. + Taking a replacing group copies the typed MessageKind, including its expression. + Taking an initial equal discriminator does not copy an unselected timer value. +- Candidate MessageKind has its own scoped draft. The independent TimerExpression + scalar draft survives group decisions; the group timer's exact raw text, editor + cursor and selection survive switching away from Timer and back. +- An overlapping kind/timer draft retains both decisions and blocks Apply until + explicitly resolved. The dialog offers **Replace timer decision with message-kind + group** and **Leave message kind unselected; use timer decision**. First equal-kind + acknowledgment can coexist with an independent timer decision. Divergent authored + candidate values reject; neither candidate silently overwrites the other. +- The kind detail includes baseline/current timer observations and the complete + proposed kind. It displays the exact plan-provided acknowledged/retired lists. + Exchange, reply-to, lifelines, annotations and pre/postconditions stay separately + scoped. Incompatible Timer contracts remain core errors; no dependency or payload + is silently removed. Existing additional-impact approval remains in force. +- Group changes compose with scalar choices and execute the exact prepared Document + batch. Tests cover both directions, all six kinds, mapped reply correlation, + historical retirement, repeated no-switch rejection, invalid combinations, + protected scopes, stale preview, source failure, comments and one-step undo/redo. +- The Traceability contextual toolbar now wraps at the available width so +Artifact + remains reachable in the compact layout. This is a single `row.wrap()` change in + `views.rs`; other toolbars are outside this slice. + +Root reported native scalar coverage on frozen integrated `e7fa061`: mixed numeric +Keep/Take/Candidate, invalid default retention/repair, zero additional changes, +atomic Apply, comment/value inspection, Undo/Redo and Save All at 1440×900 and +1100×700. Reverse/type/reference/history/reopen native checks were still continuing +when this slice was implemented. This task launched no native application. + 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. ## Remaining work -Milestone 3 must add explicit constructor closure/replacement, keyed and ordered +Milestone 3 must add the remaining constructor closure/replacement, keyed and ordered decisions, historical pairing reset, complete artifact lifecycle and multi-source review. These facts are currently inspectable, with unsupported writes visibly identified. The reference table supplies explicit mappings for scalar reference Take; it does not claim to complete a reset or establish keyed acknowledgments. -Fresh independent scalar GUI review and root-owned native verification are still -required before merging this milestone into release acceptance. In particular, -validate the 1100×700 layout, keyboard/focus continuity, long historical values, -reference selection, source reopening and prepared-impact inspection in the app. +Fresh independent requirements/correctness review and root-owned native checks of +the message-kind slice are required before release acceptance. Verify the compact +kind picker, timer text/focus retention, overlap choices, exact closure/retirement, +mapped Reply, blocked Timer contract, source reopening and toolbar wrapping.