diff --git a/crates/oa-core/src/semantic/review.rs b/crates/oa-core/src/semantic/review.rs index adb857b3..2f357179 100644 --- a/crates/oa-core/src/semantic/review.rs +++ b/crates/oa-core/src/semantic/review.rs @@ -1,5 +1,5 @@ use super::catalog::{capture_observation, collection, is_constructor, replace_observation, sides}; -use super::historical::keyed_slots_overlap; +use super::historical::{keyed_slots_overlap, selector_slots}; use super::mutate::*; use super::*; use crate::model::*; @@ -297,16 +297,22 @@ fn apply_canonical_slot( let fact = replace_observation(fact, Layer::Sa, desired.clone())?; write_fact(project, owner, &fact, Layer::Sa, Layer::Sa, false) } -fn target_only(selector: &FacetSelector, source: Layer, target: Layer) -> bool { - match selector { - FacetSelector::Member { keys, .. } | FacetSelector::Binding(keys) => { - chosen(keys, source).is_none() && chosen(keys, target).is_some() +fn member_present(fact: &FacetReview, layer: Layer) -> bool { + fn observed(review: &Reviewed, layer: Layer) -> &Observed { + match layer { + Layer::Oa => &review.oa, + Layer::Sa => &review.sa, } - FacetSelector::Literal(keys) => { - chosen(keys, source).is_none() && chosen(keys, target).is_some() + } + match fact { + FacetReview::Member { review, .. } | FacetReview::Literal { review, .. } => { + matches!(observed(review, layer), Observed::Value(true)) } - FacetSelector::Annotation(keys) => { - chosen(keys, source).is_none() && chosen(keys, target).is_some() + FacetReview::Annotation { review, .. } => { + matches!(observed(review, layer), Observed::Value(Some(_))) + } + FacetReview::Binding { review, .. } => { + matches!(observed(review, layer), Observed::Value(Some(_))) } _ => false, } @@ -320,15 +326,19 @@ fn preserve_constructor_members( source_layer: Layer, target_layer: Layer, ) -> Result<(), String> { + // A correspondence key can exist without membership on that side (for + // example an OA literal automatically projects the same SA literal key). + // Preserve actual target-only values, except previously reviewed removals. for fact in initial_facts { if !constructor_contains(group, &fact.selector) - || !target_only(&fact.selector, source_layer, target_layer) + || member_present(&fact.current, source_layer) + || !member_present(&fact.current, target_layer) { continue; } let slot = target_slot(target, &fact.selector, target_layer)?; let was_corresponding = baseline.facets.iter().any(|old| { - !target_only(&old.selector(), source_layer, target_layer) + member_present(old, source_layer) && target_slot(target, &old.selector(), target_layer).ok() == Some(slot.clone()) }); if was_corresponding { @@ -723,12 +733,28 @@ fn build_review( constructor_actions.push((target, selected.clone(), local_record)); } let refreshed = semantic_facets(observed, &trace, &references)?; - let group_selectors: BTreeSet<_> = refreshed + let mut group_selectors: BTreeSet<_> = refreshed .iter() .chain(all.iter()) .map(|f| f.selector.clone()) .filter(|f| constructor_contains(selected, f)) .collect(); + // A constructor copy can turn a live one-sided member into a + // pair. Its initial projection is not another reviewed fact. + // Established one-sided history still requires an explicit reset. + let paired_sides: BTreeSet<_> = group_selectors + .iter() + .map(selector_slots) + .filter(|slots| slots.len() == 2) + .flatten() + .collect(); + group_selectors.retain(|selector| { + !paired_sides.contains(selector) + || baseline + .facets + .iter() + .any(|old| old.selector() == *selector) + }); let mut group = vec![]; for selector in group_selectors { match capture_facet(observed, oa, sa, &selector) { diff --git a/crates/oa-core/tests/r3_semantic.rs b/crates/oa-core/tests/r3_semantic.rs index 093f5b4c..5b66d5ca 100644 --- a/crates/oa-core/tests/r3_semantic.rs +++ b/crates/oa-core/tests/r3_semantic.rs @@ -1917,3 +1917,264 @@ fn retired_source_branch_retains_historical_member_pair_in_both_directions() { ); } } + +#[test] +fn constructor_closure_preserves_target_only_literals_in_both_directions() { + for reverse in [false, true] { + let mut p = Project::empty(); + let oa = add(&mut p, Kind::DataType, Layer::Oa, "oa"); + let sa = add(&mut p, Kind::DataType, Layer::Sa, "sa"); + let (source, target) = if reverse { (sa, oa) } else { (oa, sa) }; + enumeration(&mut p, target, &["local"]); + let t = trace(&mut p, oa, sa); + let facts = semantic_facets(&p, &p.traces[&t], &BTreeMap::new()).unwrap(); + p.traces + .get_mut(&t) + .unwrap() + .baseline + .as_mut() + .unwrap() + .semantic = Some(SemanticBaseline { + facets: facts.into_iter().map(|f| f.current).collect(), + }); + enumeration(&mut p, source, &["incoming"]); + let mut r = request( + source, + target, + vec![( + FacetSelector::Data(DataFacet::Constructor), + SemanticDecision::TakeSource, + )], + ); + if reverse { + r.transition.direction = TransitionDirection::SaToOa; + } + let q = applied(&p, r); + assert!( + matches!(q.elements[&target].shared(), Some(SharedBody::DataType { + definition: DataTypeDefinition::Enumeration { literals }, .. + }) if literals == &["incoming", "local"]), + "reverse={reverse}" + ); + let base = q.traces[&t] + .baseline + .as_ref() + .unwrap() + .semantic + .as_ref() + .unwrap(); + validate_semantic_baseline(base).unwrap(); + let incoming = FacetSelector::Literal(KeyPair { + oa: Some("incoming".into()), + sa: Some("incoming".into()), + }); + assert_eq!( + base.facets + .iter() + .filter(|f| f.selector() == incoming) + .count(), + 1 + ); + let facts = semantic_facets(&q, &q.traces[&t], &BTreeMap::new()).unwrap(); + assert!( + facts + .iter() + .filter(|f| matches!(f.selector, FacetSelector::Literal(_))) + .all(|f| f.drift == Drift::Unchanged) + ); + } +} + +#[test] +fn constructor_closure_pairs_signal_members_and_requires_history_reset_in_both_directions() { + for reverse in [false, true] { + for historical in [false, true] { + let mut p = Project::empty(); + let oa = add(&mut p, Kind::Event, Layer::Oa, "oa"); + let sa = add(&mut p, Kind::Event, Layer::Sa, "sa"); + let oa_item = add(&mut p, Kind::ExchangeItem, Layer::Oa, "oa_item"); + let sa_item = add(&mut p, Kind::ExchangeItem, Layer::Sa, "sa_item"); + let (source, target, source_item) = if reverse { + (sa, oa, sa_item) + } else { + (oa, sa, oa_item) + }; + set( + &mut p, + source, + SharedBody::Event { + body: EventBody::Signal { + items: vec![source_item], + }, + }, + ); + set( + &mut p, + target, + SharedBody::Event { + body: EventBody::Change { + expression: "old".into(), + }, + }, + ); + let t = trace(&mut p, oa, sa); + let previous = FacetSelector::Member { + collection: ReferenceCollection::SignalItems, + keys: if reverse { + KeyPair { + oa: None, + sa: Some(sa_item), + } + } else { + KeyPair { + oa: Some(oa_item), + sa: None, + } + }, + }; + if historical { + let facts = semantic_facets(&p, &p.traces[&t], &BTreeMap::new()).unwrap(); + let old = facts + .into_iter() + .find(|f| f.selector == previous) + .unwrap() + .current; + p.traces + .get_mut(&t) + .unwrap() + .baseline + .as_mut() + .unwrap() + .semantic = Some(SemanticBaseline { facets: vec![old] }); + } + let mut r = request( + source, + target, + vec![( + FacetSelector::Event(EventFacet::Constructor), + SemanticDecision::TakeSource, + )], + ); + if reverse { + r.transition.direction = TransitionDirection::SaToOa; + } + r.references.insert(oa_item, sa_item); + if historical { + let invalid = plan_semantic_transition(&p, r.clone()).unwrap(); + assert!( + !invalid.conflicts.is_empty(), + "Established one-sided history requires reset: reverse={reverse}" + ); + r.reviews[0].reset.push(previous); + } + let q = applied(&p, r); + let target_item = if reverse { oa_item } else { sa_item }; + assert!( + matches!(q.elements[&target].shared(), Some(SharedBody::Event { body: EventBody::Signal { items } }) if items == &[target_item]) + ); + let base = q.traces[&t] + .baseline + .as_ref() + .unwrap() + .semantic + .as_ref() + .unwrap(); + validate_semantic_baseline(base).unwrap(); + let members: Vec<_> = base + .facets + .iter() + .filter(|f| { + matches!( + f.selector(), + FacetSelector::Member { + collection: ReferenceCollection::SignalItems, + .. + } + ) + }) + .collect(); + assert_eq!( + members.len(), + 1, + "reverse={reverse}, historical={historical}" + ); + assert_eq!( + members[0].selector(), + FacetSelector::Member { + collection: ReferenceCollection::SignalItems, + keys: KeyPair { + oa: Some(oa_item), + sa: Some(sa_item) + } + } + ); + assert!( + validation::validate(&q) + .iter() + .all(|issue| issue.severity != validation::Severity::Error) + ); + } + } +} + +#[test] +fn constructor_closure_does_not_restore_previously_corresponding_removed_members() { + for reverse in [false, true] { + let mut p = Project::empty(); + let oa = add(&mut p, Kind::DataType, Layer::Oa, "oa"); + let sa = add(&mut p, Kind::DataType, Layer::Sa, "sa"); + let (source, target) = if reverse { (sa, oa) } else { (oa, sa) }; + enumeration(&mut p, source, &["paired"]); + let t = trace(&mut p, oa, sa); + let constructor = FacetSelector::Data(DataFacet::Constructor); + let directed = |facets| { + let mut r = request(source, target, facets); + if reverse { + r.transition.direction = TransitionDirection::SaToOa; + } + r + }; + p = applied( + &p, + directed(vec![(constructor.clone(), SemanticDecision::KeepTarget)]), + ); + if reverse { + // Explicitly establish the counterpart key while its target branch + // is absent, rather than silently changing the one-sided history. + let mut r = directed(vec![( + FacetSelector::Literal(KeyPair { + oa: Some("paired".into()), + sa: Some("paired".into()), + }), + SemanticDecision::KeepTarget, + )]); + r.reviews[0].reset.push(FacetSelector::Literal(KeyPair { + oa: None, + sa: Some("paired".into()), + })); + p = applied(&p, r); + } + enumeration(&mut p, source, &["incoming"]); + enumeration(&mut p, target, &["paired", "local"]); + let q = applied( + &p, + directed(vec![(constructor, SemanticDecision::TakeSource)]), + ); + assert!( + matches!(q.elements[&target].shared(), Some(SharedBody::DataType { + definition: DataTypeDefinition::Enumeration { literals }, .. + }) if literals == &["incoming", "local"]), + "reverse={reverse}" + ); + validate_semantic_baseline( + q.traces[&t] + .baseline + .as_ref() + .unwrap() + .semantic + .as_ref() + .unwrap(), + ) + .unwrap(); + } +} diff --git a/harness/release-constructor-history-fix.md b/harness/release-constructor-history-fix.md new file mode 100644 index 00000000..bac77a9c --- /dev/null +++ b/harness/release-constructor-history-fix.md @@ -0,0 +1,28 @@ +# Constructor history correction + +Scope: `oa-core` only, based on HOST `f6a59de85b815fbea5ec578bb79d0a15819c42b4`. No desktop, source grammar, API shape, or OA port semantics changed by the keyed closure correction. + +## Reproduction and decision + +The original independent Boolean-to-Enumeration probe passes OA to SA on an unmodified HOST core. Extending the same fixture to SA to OA reproduces the duplicate `Literal(incoming,incoming)` conflict. New production-core regressions reproduce both the literal and `Event.Signal` member failures before the implementation change; `constructor-closure-red.log` records two failures. This is directional catalog projection, not UUID ordering: a live SA-only selector survives alongside the newly paired selector after a constructor copy. + +Constructor closure now removes that redundant transient one-sided projection when a pair covers it. Established historical one-sided selectors remain, so changing their correspondence still requires an explicit reset. Both-sided historical pairs and removed branch observations retain their existing rules. + +Suppressing the duplicate exposed another reverse-direction defect: target-local literals were lost because their implicit canonical key looked paired even when the source had no member. Preservation now uses observed membership (`true` or an optional row containing a value), with the existing prior source-membership guard preventing resurrection of reviewed removals. This implements the existing language.rst requirement to preserve target-only members when a historical switch reaches the target's current constructor; no normative contract was relaxed. + +The intermediate normalization-only run (`constructor-closure-normalized.log`) passes Signal pairing but fails reverse literal preservation. Three final regressions cover both directions, no-history paired Signal insertion, established one-sided history requiring reset, local literal retention, and previously corresponding removed members staying removed. A separate read-only review checked selector normalization and presence semantics; its deletion-guard test recommendation was implemented. + +## Verification + +Dedicated target: `D:/CodexBuilds/Capella2-scenario-target`. + +```powershell +$env:CARGO_TARGET_DIR='D:/CodexBuilds/Capella2-scenario-target' +cargo test --locked --offline -p oa-core --test r3_semantic +cargo test --locked --offline -p oa-core +cargo clippy --locked --offline -p oa-core --all-targets -- -D warnings +cargo fmt --all -- --check +cargo run --offline --manifest-path C:/Users/sorok/AppData/Local/Temp/capella2-constructor-literal-host-probe/Cargo.toml +``` + +All 31 semantic tests and all 356 Windows core tests pass. Strict core all-target Clippy and workspace formatting pass. The independently adapted probe passes both directions against the final implementation (`constructor-independent-green.log`). Full logs: `constructor-core-test.log`, `constructor-core-clippy.log`, `constructor-r3-semantic-green.log` in the dedicated target. Actual GUI/API acceptance and Linux integration remain with the root engineer; this core-only milestone makes no native acceptance claim.