Normalize constructor review members without losing local history

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 {
+fn member_present(fact: &FacetReview, layer: Layer) -> bool {
-    match selector {
+    fn observed<T>(review: &Reviewed<T>, layer: Layer) -> &Observed<T> {
-        FacetSelector::Member { keys, .. } | FacetSelector::Binding(keys) => {
+        match layer {
-            chosen(keys, source).is_none() && chosen(keys, target).is_some()
+            Layer::Oa => &review.oa,
+            Layer::Sa => &review.sa,
         }
-        FacetSelector::Literal(keys) => {
-            chosen(keys, source).is_none() && chosen(keys, target).is_some()
     }
-        FacetSelector::Annotation(keys) => {
+    match fact {
-            chosen(keys, source).is_none() && chosen(keys, target).is_some()
+        FacetReview::Member { review, .. } | FacetReview::Literal { review, .. } => {
+            matches!(observed(review, layer), Observed::Value(true))
+        }
+        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) {

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();
+    }
+}

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.