From 6bb2583b4aa732f3ed965789b55e2df5e2c76563 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Sun, 6 Sep 2026 06:01:48 +0300 Subject: [PATCH] Allow first review of equal constructor discriminators --- crates/oa-core/src/semantic/review.rs | 48 +- crates/oa-core/tests/r3_constructor_ack.rs | 680 ++++++++++++++++++ docs/language.html | 21 +- docs/language.rst | 22 +- .../release-constructor-acknowledgement.md | 59 ++ 5 files changed, 808 insertions(+), 22 deletions(-) create mode 100644 crates/oa-core/tests/r3_constructor_ack.rs create mode 100644 harness/release-constructor-acknowledgement.md diff --git a/crates/oa-core/src/semantic/review.rs b/crates/oa-core/src/semantic/review.rs index f1278eaf..adb857b3 100644 --- a/crates/oa-core/src/semantic/review.rs +++ b/crates/oa-core/src/semantic/review.rs @@ -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::>(); - 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(¤t, source_layer) == side(¤t, target_layer) + && (resolution.decision != SemanticDecision::Candidate + || side( + &capture_facet(&candidates, oa, sa, &resolution.facet)?, + target_layer, + ) == side(¤t, target_layer)); + Ok(( + resolution, + is_constructor(&resolution.facet) && !first_equal_constructor, + )) + }) + .collect::, 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(¤t, target_layer))?; + } + acknowledged.push(selected.clone()); + } else if constructor_group { let old = baseline.facets.iter().find(|v| v.selector() == *selected); let current_source = side(¤t, source_layer); let current_target = side(¤t, target_layer); diff --git a/crates/oa-core/tests/r3_constructor_ack.rs b/crates/oa-core/tests/r3_constructor_ack.rs new file mode 100644 index 00000000..b697b68a --- /dev/null +++ b/crates/oa-core/tests/r3_constructor_ack.rs @@ -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 { + 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()); + } +} diff --git a/docs/language.html b/docs/language.html index 8aeb2e69..8d7631ff 100644 --- a/docs/language.html +++ b/docs/language.html @@ -1825,6 +1825,14 @@ ScenarioUse, State, Fragment, Branch.

  • 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. @@ -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.

    -

    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; @@ -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.

    diff --git a/docs/language.rst b/docs/language.rst index a8a933f3..6cd41af7 100644 --- a/docs/language.rst +++ b/docs/language.rst @@ -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. diff --git a/harness/release-constructor-acknowledgement.md b/harness/release-constructor-acknowledgement.md new file mode 100644 index 00000000..7063d802 --- /dev/null +++ b/harness/release-constructor-acknowledgement.md @@ -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.