Allow first review of equal constructor discriminators

2026-09-06 06:01:48 +03:00
parent d130013155
commit 6bb2583b4a
5 changed files with 808 additions and 22 deletions
@@ -562,8 +562,32 @@ fn build_review(
        let all = semantic_facets(initial, &trace, &references)?;
        let mut acknowledged = vec![];
        let mut retired = vec![];
-        let mut resolutions = request.resolutions.iter().collect::<Vec<_>>();
-        for (i, resolution) in resolutions.iter().enumerate() {
+        let mut resolutions = request
+            .resolutions
+            .iter()
+            .map(|resolution| {
+                let current = capture_facet(initial, oa, sa, &resolution.facet)?;
+                // A first equal discriminator observation has only its scalar
+                // scope. Existing history and actual proposals retain the full
+                // constructor group, including its no-switch rejection.
+                let first_equal_constructor = is_constructor(&resolution.facet)
+                    && !baseline
+                        .facets
+                        .iter()
+                        .any(|old| old.selector() == resolution.facet)
+                    && side(&current, source_layer) == side(&current, target_layer)
+                    && (resolution.decision != SemanticDecision::Candidate
+                        || side(
+                            &capture_facet(&candidates, oa, sa, &resolution.facet)?,
+                            target_layer,
+                        ) == side(&current, target_layer));
+                Ok((
+                    resolution,
+                    is_constructor(&resolution.facet) && !first_equal_constructor,
+                ))
+            })
+            .collect::<Result<Vec<_>, String>>()?;
+        for (i, (resolution, constructor_group)) in resolutions.iter().enumerate() {
            check_keys(&resolution.facet, &references)?;
            if let FacetSelector::Artifact { keys, facet } = &resolution.facet
                && *facet != ArtifactFacet::Presence
@@ -577,11 +601,10 @@ fn build_review(
                    }
                }
            }
-            capture_facet(initial, oa, sa, &resolution.facet)?;
-            if resolutions[..i].iter().any(|r| {
+            if resolutions[..i].iter().any(|(r, previous_group)| {
                keyed_slots_overlap(&r.facet, &resolution.facet)
-                    || constructor_contains(&r.facet, &resolution.facet)
-                    || constructor_contains(&resolution.facet, &r.facet)
+                    || (*previous_group && constructor_contains(&r.facet, &resolution.facet))
+                    || (*constructor_group && constructor_contains(&resolution.facet, &r.facet))
            }) {
                return Err("Duplicate, colliding or overlapping resolution scopes".into());
            }
@@ -611,8 +634,8 @@ fn build_review(
                }
            }
        }
-        resolutions.sort_by_key(|r| {
-            if is_constructor(&r.facet) {
+        resolutions.sort_by_key(|(r, constructor_group)| {
+            if *constructor_group {
                0
            } else if matches!(r.facet, FacetSelector::Order(_)) {
                2
@@ -620,10 +643,15 @@ fn build_review(
                1
            }
        });
-        for resolution in resolutions {
+        for (resolution, constructor_group) in resolutions {
            let selected = &resolution.facet;
            let current = capture_facet(initial, oa, sa, selected)?;
-            if is_constructor(selected) {
+            if is_constructor(selected) && !constructor_group {
+                if let Some(slot) = target_slot(target, selected, target_layer)? {
+                    agree(&mut slots, slot, side(&current, target_layer))?;
+                }
+                acknowledged.push(selected.clone());
+            } else if constructor_group {
                let old = baseline.facets.iter().find(|v| v.selector() == *selected);
                let current_source = side(&current, source_layer);
                let current_target = side(&current, target_layer);
@@ -0,0 +1,680 @@
+use oa_core::{transitions::*, *};
+use std::collections::BTreeMap;
+
+#[derive(Clone, Copy, Debug)]
+enum Family {
+    Data,
+    Port,
+    Occurrence,
+    Message,
+    Event,
+    State,
+}
+const FAMILIES: [Family; 6] = [
+    Family::Data,
+    Family::Port,
+    Family::Occurrence,
+    Family::Message,
+    Family::Event,
+    Family::State,
+];
+
+fn add(p: &mut Project, kind: Kind, layer: Layer, parent: Option<Id>) -> Id {
+    p.add_element(kind, layer, "reviewed", parent).unwrap()
+}
+fn trace(p: &mut Project, oa: Id, sa: Id) -> Id {
+    let id = Id::new_v4();
+    p.traces.insert(
+        id,
+        Trace {
+            id,
+            alias: format!("review_{}", id.simple()),
+            name: "Review".into(),
+            description: String::new(),
+            oa,
+            sa,
+            kind: TraceKind::Manual,
+            baseline: Some(TraceBaseline {
+                oa: p.properties(oa).unwrap(),
+                sa: p.properties(sa).unwrap(),
+                semantic: None,
+            }),
+        },
+    );
+    id
+}
+fn set(p: &mut Project, id: Id, body: SharedBody) {
+    *p.elements.get_mut(&id).unwrap().shared_mut().unwrap() = body;
+}
+fn integer(p: &mut Project, id: Id) {
+    set(
+        p,
+        id,
+        SharedBody::DataType {
+            definition: DataTypeDefinition::Integer {
+                minimum: Some(0),
+                maximum: Some(10),
+            },
+            default: None,
+            unit: "m".into(),
+        },
+    );
+}
+fn fixture(family: Family) -> (Project, Id, Id, Id, FacetSelector, FacetSelector) {
+    let mut p = Project::empty();
+    let mut pair = vec![];
+    for layer in [Layer::Oa, Layer::Sa] {
+        let id = match family {
+            Family::Data => {
+                let id = add(&mut p, Kind::DataType, layer, None);
+                integer(&mut p, id);
+                id
+            }
+            Family::Port => {
+                let kind = if layer == Layer::Oa {
+                    Kind::Actor
+                } else {
+                    Kind::SystemActor
+                };
+                let owner = add(&mut p, kind, layer, None);
+                let id = Id::new_v4();
+                let mut port = make_port(&p, id, owner, "port", PortContract::Legacy).unwrap();
+                if let Some(SharedBody::Port { legacy, .. }) = port.shared_mut() {
+                    *legacy = Some(LegacyPortContract {
+                        direction: PortDirection::InOut,
+                        interface: None,
+                    });
+                }
+                p.elements.insert(id, port);
+                id
+            }
+            Family::Occurrence | Family::Message => {
+                let owner = add(&mut p, Kind::Scenario, layer, None);
+                let id = add(&mut p, Kind::Occurrence, layer, Some(owner));
+                p.elements
+                    .get_mut(&owner)
+                    .unwrap()
+                    .steps_mut()
+                    .unwrap()
+                    .push(id);
+                set(
+                    &mut p,
+                    id,
+                    SharedBody::Occurrence {
+                        body: OccurrenceBody::Exchange {
+                            exchange: None,
+                            sender: None,
+                            receiver: None,
+                            message_kind: MessageKind::Timer {
+                                expression: "10s".into(),
+                            },
+                            reply_to: None,
+                            values: vec![],
+                        },
+                        precondition: String::new(),
+                        postcondition: String::new(),
+                    },
+                );
+                id
+            }
+            Family::Event => {
+                let id = add(&mut p, Kind::Event, layer, None);
+                set(
+                    &mut p,
+                    id,
+                    SharedBody::Event {
+                        body: EventBody::Time {
+                            kind: TimeEventKind::After,
+                            expression: "10s".into(),
+                        },
+                    },
+                );
+                id
+            }
+            Family::State => {
+                let machine = add(&mut p, Kind::StateMachine, layer, None);
+                let region = add(&mut p, Kind::Region, layer, Some(machine));
+                add(&mut p, Kind::State, layer, Some(region))
+            }
+        };
+        pair.push(id);
+    }
+    let (oa, sa) = (pair[0], pair[1]);
+    let t = trace(&mut p, oa, sa);
+    let (constructor, descendant) = match family {
+        Family::Data => (
+            FacetSelector::Data(DataFacet::Constructor),
+            FacetSelector::Data(DataFacet::IntegerMaximum),
+        ),
+        Family::Port => (
+            FacetSelector::Port(PortFacet::Constructor),
+            FacetSelector::Port(PortFacet::LegacyDirection),
+        ),
+        Family::Occurrence => (
+            FacetSelector::Occurrence(OccurrenceFacet::Constructor),
+            FacetSelector::Occurrence(OccurrenceFacet::TimerExpression),
+        ),
+        Family::Message => (
+            FacetSelector::Occurrence(OccurrenceFacet::MessageKind),
+            FacetSelector::Occurrence(OccurrenceFacet::TimerExpression),
+        ),
+        Family::Event => (
+            FacetSelector::Event(EventFacet::Constructor),
+            FacetSelector::Event(EventFacet::TimeExpression),
+        ),
+        Family::State => (
+            FacetSelector::State(StateFacet::Kind),
+            FacetSelector::State(StateFacet::Entry),
+        ),
+    };
+    (p, oa, sa, t, constructor, descendant)
+}
+fn change_descendant(p: &mut Project, id: Id) {
+    match p.elements.get_mut(&id).unwrap().shared_mut().unwrap() {
+        SharedBody::DataType {
+            definition: DataTypeDefinition::Integer { maximum, .. },
+            ..
+        } => *maximum = Some(20),
+        SharedBody::Port {
+            legacy: Some(LegacyPortContract { direction, .. }),
+            ..
+        } => *direction = PortDirection::Out,
+        SharedBody::Occurrence {
+            body:
+                OccurrenceBody::Exchange {
+                    message_kind: MessageKind::Timer { expression },
+                    ..
+                },
+            ..
+        }
+        | SharedBody::Event {
+            body: EventBody::Time { expression, .. },
+        } => *expression = "20s".into(),
+        SharedBody::State { entry, .. } => *entry = "initialize".into(),
+        _ => panic!("Unexpected test family"),
+    }
+}
+fn request(
+    oa: Id,
+    sa: Id,
+    reverse: bool,
+    resolutions: Vec<(FacetSelector, SemanticDecision)>,
+) -> SemanticTransitionRequest {
+    let (direction, source, target) = if reverse {
+        (TransitionDirection::SaToOa, sa, oa)
+    } else {
+        (TransitionDirection::OaToSa, oa, sa)
+    };
+    let mut transition = TransitionRequest::new(direction, vec![source]);
+    transition.mappings.push(TransitionMapping {
+        source,
+        target: Some(target),
+        target_kind: None,
+    });
+    let mut r = SemanticTransitionRequest::new(transition);
+    r.reviews.push(SemanticReviewRequest {
+        source,
+        resolutions: resolutions
+            .into_iter()
+            .map(|(facet, decision)| FacetResolution { facet, decision })
+            .collect(),
+        reset: vec![],
+    });
+    r
+}
+fn apply(p: &Project, r: SemanticTransitionRequest) -> (Project, SemanticTransitionPlan) {
+    let plan = plan_semantic_transition(p, r).unwrap();
+    assert!(plan.conflicts.is_empty(), "{:?}", plan.conflicts);
+    (apply_semantic_transition(p, &plan).unwrap(), plan)
+}
+fn drift(p: &Project, t: Id, selector: &FacetSelector) -> Drift {
+    semantic_facets(p, &p.traces[&t], &BTreeMap::new())
+        .unwrap()
+        .into_iter()
+        .find(|f| f.selector == *selector)
+        .unwrap()
+        .drift
+}
+
+#[test]
+fn first_equal_constructor_acknowledges_only_discriminator_in_every_family_and_direction() {
+    for family in FAMILIES {
+        for reverse in [false, true] {
+            for decision in [
+                SemanticDecision::KeepTarget,
+                SemanticDecision::TakeSource,
+                SemanticDecision::Candidate,
+            ] {
+                let (mut p, oa, sa, t, constructor, descendant) = fixture(family);
+                let (source, target) = if reverse { (sa, oa) } else { (oa, sa) };
+                change_descendant(&mut p, target);
+                let mut r = request(oa, sa, reverse, vec![(constructor.clone(), decision)]);
+                if decision == SemanticDecision::Candidate {
+                    r.candidates
+                        .push(SemanticRecord::Element(p.elements[&target].clone()));
+                }
+                let (q, plan) = apply(&p, r.clone());
+                assert_eq!(q.elements, p.elements, "{family:?} {decision:?}");
+                assert_eq!(plan.reviews[0].acknowledged, vec![constructor.clone()]);
+                assert!(plan.reviews[0].retired.is_empty());
+                let facts = semantic_facets(&q, &q.traces[&t], &BTreeMap::new()).unwrap();
+                assert!(facts.iter().all(|f| f.drift
+                    == if f.selector == constructor {
+                        Drift::Unchanged
+                    } else {
+                        Drift::Unreviewed
+                    }));
+                let (mut q, _) = apply(
+                    &q,
+                    request(
+                        oa,
+                        sa,
+                        reverse,
+                        vec![(descendant.clone(), SemanticDecision::KeepTarget)],
+                    ),
+                );
+                change_descendant(&mut q, source);
+                assert_eq!(
+                    drift(&q, t, &descendant),
+                    if reverse {
+                        Drift::SaChanged
+                    } else {
+                        Drift::OaChanged
+                    }
+                );
+                let repeated = plan_semantic_transition(&q, r).unwrap();
+                assert!(
+                    repeated
+                        .conflicts
+                        .iter()
+                        .any(|c| c.contains("No constructor switch"))
+                );
+                assert!(apply_semantic_transition(&q, &repeated).is_err());
+            }
+        }
+    }
+}
+
+#[test]
+fn independent_constructor_and_leaf_selections_complete_missing_or_empty_baselines() {
+    for family in FAMILIES {
+        for reverse in [false, true] {
+            for history in 0..3 {
+                let (mut p, oa, sa, t, _, _) = fixture(family);
+                if history == 0 {
+                    p.traces.get_mut(&t).unwrap().baseline = None;
+                } else if history == 2 {
+                    p.traces
+                        .get_mut(&t)
+                        .unwrap()
+                        .baseline
+                        .as_mut()
+                        .unwrap()
+                        .semantic = Some(SemanticBaseline::default());
+                }
+                let selectors: Vec<_> = semantic_facets(&p, &p.traces[&t], &BTreeMap::new())
+                    .unwrap()
+                    .into_iter()
+                    .map(|f| f.selector)
+                    .collect();
+                let (q, plan) = apply(
+                    &p,
+                    request(
+                        oa,
+                        sa,
+                        reverse,
+                        selectors
+                            .iter()
+                            .cloned()
+                            .map(|s| (s, SemanticDecision::KeepTarget))
+                            .collect(),
+                    ),
+                );
+                assert_eq!(q.elements, p.elements);
+                assert_eq!(plan.reviews[0].acknowledged.len(), selectors.len());
+                assert!(plan.reviews[0].retired.is_empty());
+                let status = trace_semantic_drift(&q, &q.traces[&t], &BTreeMap::new()).unwrap();
+                assert_eq!(status.own, Drift::Unchanged, "{family:?} {history}");
+                assert!(!status.own_unreviewed);
+            }
+        }
+    }
+}
+
+#[test]
+fn migrated_equal_constructor_can_complete_semantic_review_without_model_changes() {
+    let json = include_str!("fixtures/v4-reviewed-prose.json")
+        .replace("\"Real\"", "\"Integer\"")
+        .replace("0.0", "0")
+        .replace("100.0", "100")
+        .replace("20.0", "20");
+    for reverse in [false, true] {
+        let decoded = persistence::decode_with_report(json.as_bytes()).unwrap();
+        let report = decoded.migration.unwrap();
+        assert_eq!((report.from_schema, report.to_schema), (4, 5));
+        let p = decoded.project;
+        let t = p.traces.values().next().unwrap();
+        assert!(t.baseline.as_ref().unwrap().semantic.is_none());
+        let original_prose = t.baseline.clone().unwrap();
+        let (oa, sa, id) = (t.oa, t.sa, t.id);
+        let selections = semantic_facets(&p, t, &BTreeMap::new())
+            .unwrap()
+            .into_iter()
+            .map(|f| (f.selector, SemanticDecision::KeepTarget))
+            .collect();
+        let (q, _) = apply(&p, request(oa, sa, reverse, selections));
+        assert_eq!(q.elements, p.elements);
+        let baseline = q.traces[&id].baseline.as_ref().unwrap();
+        assert_eq!(baseline.oa, original_prose.oa);
+        assert_eq!(baseline.sa, original_prose.sa);
+        let status = trace_semantic_drift(&q, &q.traces[&id], &BTreeMap::new()).unwrap();
+        assert_eq!(status.own, Drift::Unchanged);
+        assert!(!status.own_unreviewed);
+    }
+}
+
+#[test]
+fn equal_constructor_candidate_requires_independent_selection_of_changed_descendants() {
+    for reverse in [false, true] {
+        let (p, oa, sa, t, constructor, descendant) = fixture(Family::Data);
+        let target = if reverse { oa } else { sa };
+        let mut changed = p.clone();
+        change_descendant(&mut changed, target);
+        let mut r = request(
+            oa,
+            sa,
+            reverse,
+            vec![(constructor.clone(), SemanticDecision::Candidate)],
+        );
+        r.candidates
+            .push(SemanticRecord::Element(changed.elements[&target].clone()));
+        let invalid = plan_semantic_transition(&p, r.clone()).unwrap();
+        assert!(
+            invalid
+                .conflicts
+                .iter()
+                .any(|c| c.contains("changes an unselected facet"))
+        );
+        assert!(apply_semantic_transition(&p, &invalid).is_err());
+        r.reviews[0].resolutions.push(FacetResolution {
+            facet: descendant.clone(),
+            decision: SemanticDecision::Candidate,
+        });
+        let (q, plan) = apply(&p, r);
+        assert_eq!(q.elements, changed.elements);
+        assert_eq!(plan.reviews[0].acknowledged.len(), 2);
+        assert_eq!(drift(&q, t, &constructor), Drift::Unchanged);
+        assert_eq!(drift(&q, t, &descendant), Drift::Unchanged);
+        assert_eq!(
+            drift(&q, t, &FacetSelector::Data(DataFacet::Default)),
+            Drift::Unreviewed
+        );
+    }
+}
+
+#[test]
+fn reset_equal_constructor_does_not_refresh_previously_reviewed_descendants() {
+    for reverse in [false, true] {
+        let (p, oa, sa, t, constructor, descendant) = fixture(Family::Data);
+        let (mut p, _) = apply(
+            &p,
+            request(
+                oa,
+                sa,
+                reverse,
+                vec![
+                    (constructor.clone(), SemanticDecision::KeepTarget),
+                    (descendant.clone(), SemanticDecision::KeepTarget),
+                ],
+            ),
+        );
+        let original = p.traces[&t]
+            .baseline
+            .as_ref()
+            .unwrap()
+            .semantic
+            .as_ref()
+            .unwrap()
+            .clone();
+        change_descendant(&mut p, if reverse { sa } else { oa });
+        let mut r = request(
+            oa,
+            sa,
+            reverse,
+            vec![(constructor.clone(), SemanticDecision::KeepTarget)],
+        );
+        r.reviews[0].reset.push(constructor.clone());
+        let (q, plan) = apply(&p, r);
+        assert_eq!(q.elements, p.elements);
+        assert_eq!(plan.reviews[0].acknowledged, vec![constructor]);
+        assert!(plan.reviews[0].retired.is_empty());
+        assert_eq!(
+            q.traces[&t]
+                .baseline
+                .as_ref()
+                .unwrap()
+                .semantic
+                .as_ref()
+                .unwrap(),
+            &original
+        );
+        assert_eq!(
+            drift(&q, t, &descendant),
+            if reverse {
+                Drift::SaChanged
+            } else {
+                Drift::OaChanged
+            }
+        );
+    }
+}
+
+#[test]
+fn first_constructor_acknowledgement_agrees_only_on_its_canonical_discriminator_slot() {
+    for switch in [false, true] {
+        let (mut p, oa, sa, t, constructor, descendant) = fixture(Family::Data);
+        let second = add(&mut p, Kind::DataType, Layer::Oa, None);
+        integer(&mut p, second);
+        if switch {
+            set(
+                &mut p,
+                second,
+                SharedBody::DataType {
+                    definition: DataTypeDefinition::Real {
+                        minimum: Some(0.),
+                        maximum: Some(30.),
+                    },
+                    default: None,
+                    unit: "m".into(),
+                },
+            );
+        } else {
+            change_descendant(&mut p, second);
+        }
+        trace(&mut p, second, sa);
+        let mut r = request(
+            oa,
+            sa,
+            false,
+            vec![(constructor.clone(), SemanticDecision::KeepTarget)],
+        );
+        r.transition.sources.push(second);
+        r.transition.mappings.push(TransitionMapping {
+            source: second,
+            target: Some(sa),
+            target_kind: None,
+        });
+        r.reviews.push(SemanticReviewRequest {
+            source: second,
+            reset: vec![],
+            resolutions: vec![FacetResolution {
+                facet: if switch {
+                    constructor.clone()
+                } else {
+                    descendant.clone()
+                },
+                decision: SemanticDecision::TakeSource,
+            }],
+        });
+        for reversed in [false, true] {
+            let mut r = r.clone();
+            if reversed {
+                r.reviews.reverse();
+            }
+            let plan = plan_semantic_transition(&p, r).unwrap();
+            if switch {
+                assert!(plan.conflicts.iter().any(|c| c.contains("Contradictory")));
+                assert!(apply_semantic_transition(&p, &plan).is_err());
+            } else {
+                assert!(plan.conflicts.is_empty(), "{:?}", plan.conflicts);
+                let q = apply_semantic_transition(&p, &plan).unwrap();
+                assert_eq!(drift(&q, t, &constructor), Drift::Unchanged);
+                assert_eq!(drift(&q, t, &descendant), Drift::Unreviewed);
+                assert!(matches!(
+                    q.elements[&sa].shared(),
+                    Some(SharedBody::DataType {
+                        definition: DataTypeDefinition::Integer {
+                            maximum: Some(20),
+                            ..
+                        },
+                        ..
+                    })
+                ));
+            }
+        }
+    }
+}
+
+#[test]
+fn one_sided_or_disappeared_message_constructor_is_not_an_equal_scalar_acknowledgement() {
+    for reverse in [false, true] {
+        let (mut p, oa, sa, t, constructor, descendant) = fixture(Family::Message);
+        let source = if reverse { sa } else { oa };
+        let target = if reverse { oa } else { sa };
+        let mut action = p.elements[&source].shared().unwrap().clone();
+        if let SharedBody::Occurrence { body, .. } = &mut action {
+            *body = OccurrenceBody::Action {
+                action: None,
+                lifeline: None,
+            };
+        }
+        set(&mut p, source, action.clone());
+        let (q, plan) = apply(
+            &p,
+            request(
+                oa,
+                sa,
+                reverse,
+                vec![(constructor.clone(), SemanticDecision::KeepTarget)],
+            ),
+        );
+        // One-sided applicability still has a constructor-group scope and records its timer leaf.
+        assert_eq!(
+            plan.reviews[0].acknowledged,
+            vec![constructor.clone(), descendant.clone()]
+        );
+        assert_eq!(q.elements, p.elements);
+        assert_eq!(drift(&q, t, &descendant), Drift::Unchanged);
+        let mut p = q;
+        set(&mut p, target, action);
+        let mut r = request(
+            oa,
+            sa,
+            reverse,
+            vec![(constructor.clone(), SemanticDecision::KeepTarget)],
+        );
+        r.reviews[0].reset.push(constructor);
+        let plan = plan_semantic_transition(&p, r).unwrap();
+        assert!(!plan.conflicts.is_empty());
+        assert!(apply_semantic_transition(&p, &plan).is_err());
+    }
+}
+
+#[test]
+fn historical_switches_retain_groups_and_completed_requests_still_reject_in_both_directions() {
+    for reverse in [false, true] {
+        let (p, oa, sa, t, constructor, _) = fixture(Family::Data);
+        let (source, target) = if reverse { (sa, oa) } else { (oa, sa) };
+        let keep = request(
+            oa,
+            sa,
+            reverse,
+            vec![(constructor.clone(), SemanticDecision::KeepTarget)],
+        );
+        let (mut p, _) = apply(&p, keep.clone());
+        set(
+            &mut p,
+            source,
+            SharedBody::DataType {
+                definition: DataTypeDefinition::Real {
+                    minimum: Some(0.),
+                    maximum: Some(20.),
+                },
+                default: None,
+                unit: "m".into(),
+            },
+        );
+        let (mut q, plan) = apply(&p, keep.clone());
+        let maximum = FacetSelector::Data(DataFacet::RealMaximum);
+        assert!(plan.reviews[0].acknowledged.contains(&maximum));
+        assert!(
+            plan.reviews[0]
+                .acknowledged
+                .contains(&FacetSelector::Data(DataFacet::Default))
+        );
+        assert!(
+            !plan.reviews[0]
+                .acknowledged
+                .contains(&FacetSelector::Data(DataFacet::Unit))
+        );
+        assert_eq!(q.elements, p.elements);
+        if let Some(SharedBody::DataType {
+            definition: DataTypeDefinition::Real { maximum, .. },
+            ..
+        }) = q.elements.get_mut(&source).unwrap().shared_mut()
+        {
+            *maximum = Some(30.);
+        }
+        let repeated = plan_semantic_transition(&q, keep).unwrap();
+        assert!(
+            repeated
+                .conflicts
+                .iter()
+                .any(|c| c.contains("No constructor switch"))
+        );
+        assert!(apply_semantic_transition(&q, &repeated).is_err());
+        assert_eq!(
+            drift(&q, t, &maximum),
+            if reverse {
+                Drift::SaChanged
+            } else {
+                Drift::OaChanged
+            }
+        );
+        let take = request(
+            oa,
+            sa,
+            reverse,
+            vec![(constructor, SemanticDecision::TakeSource)],
+        );
+        let (p, _) = apply(&q, take.clone());
+        assert!(matches!(
+            p.elements[&target].shared(),
+            Some(SharedBody::DataType {
+                definition: DataTypeDefinition::Real {
+                    maximum: Some(30.),
+                    ..
+                },
+                ..
+            })
+        ));
+        let completed = plan_semantic_transition(&p, take).unwrap();
+        assert!(
+            completed
+                .conflicts
+                .iter()
+                .any(|c| c.contains("No constructor switch"))
+        );
+        assert!(apply_semantic_transition(&p, &completed).is_err());
+    }
+}
@@ -1825,6 +1825,14 @@ ScenarioUse, State, Fragment, Branch.</p></li>
 <li><p>EventConstructor: Signal, Time, Change.</p></li>
 <li><p>MessageConstructor: Asynchronous, Call, Reply, Create, Delete, Timer.</p></li>
 </ul>
+<p>When a discriminator has no prior fact after explicit resets, both current sides
+are applicable and equal, and the proposed target discriminator is unchanged,
+KeepTarget, TakeSource or Candidate acknowledges only that discriminator.
+It MUST NOT copy a constructor payload, acknowledge unselected descendants or
+retire branch history. Descendant facets may be independently selected in the
+same review; their decisions retain their own scopes and final-value audits.
+Candidate records still MUST NOT change an unselected facet. Missing applicability
+on either side does not qualify as an equal discriminator acknowledgement.</p>
 <p>A grouped constructor resolution uses a transient typed target record, then
 records the discriminator and each acknowledged descendant as separate facts.
 It MUST NOT persist a second whole constructor payload overlapping those leaves.
@@ -1835,9 +1843,9 @@ order, annotations and bindings, but excludes outer Precondition/Postcondition.
 MessageKind includes its discriminator and dependent TimerExpression. State.Kind
 is its own structural facet even though it changes the element's derived kind.
 Other dependent changes caused by preparation remain explicit impacts.</p>
-<p>A constructor Keep requires an actual discriminator proposal or a discriminator
-change since its prior review. Repeating Keep on an already reviewed unequal pair
-MUST NOT acknowledge unrelated bound/default drift. A real target switch may
+<p>A grouped constructor Keep requires an actual discriminator proposal or a
+discriminator change since its prior review. Repeating Keep on an already reviewed
+unequal pair MUST NOT acknowledge unrelated bound/default drift. A real target switch may
 reconsider a previously kept difference. When the target already has the proposed
 constructor, reconciliation MUST preserve target-only members rather than replace
 its whole list. Removed branch facts participate in agreement as NotApplicable;
@@ -1903,9 +1911,10 @@ not arbitrary architectural neighborhoods, and does not copy them into every
 referrer's baseline. Its limits are 4096 objects per side and 32768 edges; exhaustion
 is a diagnostic. Successful review remains one undoable transaction. Replanning
 unchanged, already acknowledged leaf facts is a no-op only while their requests
-remain applicable. A completed constructor switch MUST be removed from subsequent
-requests; repeating it when no switch remains is rejected. After an artifact moves
-to another owner, metadata review through the old owner's trace is no longer
+remain applicable. A completed constructor switch or first acknowledgement of an
+equal discriminator MUST be removed from subsequent requests; repeating it when
+no switch remains is rejected. After an artifact moves to another owner,
+metadata review through the old owner's trace is no longer
 applicable and is rejected. An old plan becomes stale after an actual committed
 change.</p>
 </section>
@@ -1155,6 +1155,15 @@ Persisted constructors are discriminator words only:
 * EventConstructor: Signal, Time, Change.
 * MessageConstructor: Asynchronous, Call, Reply, Create, Delete, Timer.

+When a discriminator has no prior fact after explicit resets, both current sides
+are applicable and equal, and the proposed target discriminator is unchanged,
+KeepTarget, TakeSource or Candidate acknowledges only that discriminator.
+It MUST NOT copy a constructor payload, acknowledge unselected descendants or
+retire branch history. Descendant facets may be independently selected in the
+same review; their decisions retain their own scopes and final-value audits.
+Candidate records still MUST NOT change an unselected facet. Missing applicability
+on either side does not qualify as an equal discriminator acknowledgement.
+
 A grouped constructor resolution uses a transient typed target record, then
 records the discriminator and each acknowledged descendant as separate facts.
 It MUST NOT persist a second whole constructor payload overlapping those leaves.
@@ -1166,9 +1175,9 @@ MessageKind includes its discriminator and dependent TimerExpression. State.Kind
 is its own structural facet even though it changes the element's derived kind.
 Other dependent changes caused by preparation remain explicit impacts.

-A constructor Keep requires an actual discriminator proposal or a discriminator
-change since its prior review. Repeating Keep on an already reviewed unequal pair
-MUST NOT acknowledge unrelated bound/default drift. A real target switch may
+A grouped constructor Keep requires an actual discriminator proposal or a
+discriminator change since its prior review. Repeating Keep on an already reviewed
+unequal pair MUST NOT acknowledge unrelated bound/default drift. A real target switch may
 reconsider a previously kept difference. When the target already has the proposed
 constructor, reconciliation MUST preserve target-only members rather than replace
 its whole list. Removed branch facts participate in agreement as NotApplicable;
@@ -1240,9 +1249,10 @@ not arbitrary architectural neighborhoods, and does not copy them into every
 referrer's baseline. Its limits are 4096 objects per side and 32768 edges; exhaustion
 is a diagnostic. Successful review remains one undoable transaction. Replanning
 unchanged, already acknowledged leaf facts is a no-op only while their requests
-remain applicable. A completed constructor switch MUST be removed from subsequent
-requests; repeating it when no switch remains is rejected. After an artifact moves
-to another owner, metadata review through the old owner's trace is no longer
+remain applicable. A completed constructor switch or first acknowledgement of an
+equal discriminator MUST be removed from subsequent requests; repeating it when
+no switch remains is rejected. After an artifact moves to another owner,
+metadata review through the old owner's trace is no longer
 applicable and is rejected. An old plan becomes stale after an actual committed
 change.

@@ -0,0 +1,59 @@
+# First review of equal constructor discriminators
+
+The review planner previously rejected every unchanged-constructor decision even
+when that discriminator had never been reviewed. The catalog correctly retained
+it as Unreviewed. An isolated executable against core346fb24 reproduced an ordinary
+DataType review that acknowledged all seven other facets but could not complete
+its own baseline. The same result occurred with no outer baseline, semantic=None,
+an empty semantic baseline, and a schema4 trace migrated to schema5. KeepTarget,
+TakeSource and an unchanged Candidate all returned "No constructor switch".
+Explicitly inserting only the valid discriminator fact made own drift Unchanged.
+This was a material completion defect for existing and migrated review workflows,
+without evidence of model corruption. Direct source editing of a valid baseline
+or an actual constructor switch were possible workarounds, not normal initial
+review behavior.
+
+The agreed fix preserves the public request and stored schema. After explicit
+resets, a constructor with no historical fact has scalar scope if both current
+sides are applicable and equal and the proposed target discriminator is unchanged.
+KeepTarget, equal TakeSource and an unchanged-discriminator Candidate then record
+only that discriminator. They do not copy a constructor payload, expand descendants,
+retire historical branch facts or change model records.
+
+Resolution overlap checks use this effective scope, so separately chosen leaves
+can accompany the first discriminator acknowledgement. Its exact canonical target
+slot enters the existing agreement map and both final-value audits. A different
+source may independently change a descendant, but cannot contradict the retained
+discriminator with a constructor switch. Final Candidate record equality still
+rejects changes to unselected fields.
+
+Real proposals and historical discriminator changes retain the existing grouped
+scope. Repeated requests after the first acknowledgement or a completed switch
+still reject when no switch remains. Repeated Keep on a reviewed unequal pair
+cannot hide new bound/default drift. One-sided applicability stays on the group
+path; a discriminator inapplicable on both sides cannot be acknowledged, even
+after resetting its old fact. This avoids automatic history fabrication and does
+not suppress missing facts from the drift rollup.
+
+Normative source5 text and its HTML copy now distinguish the first scalar
+acknowledgement from grouped constructor decisions. No source syntax, persisted
+baseline type, API request shape, migration rule or desktop code changed here.
+
+Verification uses D:/CodexBuilds/Capella2-semantic-target exclusively. The eight
+new regressions cover all constructor families (valid Legacy OA/SA ports), both
+directions and all decisions; missing/empty/migrated history; simultaneous complete
+review; untouched and later changed descendants; reset; Candidate scope; canonical
+agreement in either source order; one-sided/disappeared MessageKind; and repeated
+unequal Keep/completed-switch rejection. The focused run passed all eight new
+tests and all 28 existing r3_semantic tests. Final verification passed all 353 core
+tests (33 test summaries, including doc tests) and strict Clippy for every core
+target:
+
+```
+cargo test -p oa-core --locked --offline --target-dir D:/CodexBuilds/Capella2-semantic-target
+cargo clippy -p oa-core --all-targets --locked --offline --target-dir D:/CodexBuilds/Capella2-semantic-target -- -D warnings
+```
+
+Logs are in C:/Users/sorok/AppData/Local/Temp/capella2-constructor-ack-review/
+as full-core.log and strict-clippy.log. Native/API release acceptance is outside
+this bounded fix.