From 28ebbbbc4cc358c8c86f80e85420efb2c50d0f00 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Sun, 6 Sep 2026 07:39:04 +0300 Subject: [PATCH] Allow review of explicitly mapped existing OA Legacy ports --- crates/oa-core/src/transitions.rs | 21 +++- crates/oa-core/tests/r3_semantic.rs | 131 +++++++++++++++++++++ harness/release-constructor-history-fix.md | 19 +++ 3 files changed, 170 insertions(+), 1 deletion(-) diff --git a/crates/oa-core/src/transitions.rs b/crates/oa-core/src/transitions.rs index bf44464b..e57e10a2 100644 --- a/crates/oa-core/src/transitions.rs +++ b/crates/oa-core/src/transitions.rs @@ -395,11 +395,30 @@ pub fn plan_transition( commands: vec![], }; let mut selected: BTreeSet<_> = request.sources.iter().copied().collect(); - let omitted = if request.direction == TransitionDirection::SaToOa { + let mut omitted = if request.direction == TransitionDirection::SaToOa { sa_port_omissions(project) } else { BTreeSet::new() }; + // An explicit existing Legacy counterpart can be compared and reviewed + // without inventing an OA typed port. Only this port is retained: dependent + // SA implementation records and new counterpart requests stay omitted. + for mapping in &request.mappings { + if project + .elements + .get(&mapping.source) + .is_some_and(|e| e.layer() == Layer::Sa && crate::ports::port_contract(e).is_some()) + && mapping + .target + .and_then(|target| project.elements.get(&target)) + .is_some_and(|e| { + e.layer() == Layer::Oa + && crate::ports::port_contract(e) == Some(&PortContract::Legacy) + }) + { + omitted.remove(&mapping.source); + } + } let mut reported_omissions = BTreeSet::new(); if selected.len() != request.sources.len() { conflict( diff --git a/crates/oa-core/tests/r3_semantic.rs b/crates/oa-core/tests/r3_semantic.rs index 5b66d5ca..1e0ef9fe 100644 --- a/crates/oa-core/tests/r3_semantic.rs +++ b/crates/oa-core/tests/r3_semantic.rs @@ -2178,3 +2178,134 @@ fn constructor_closure_does_not_restore_previously_corresponding_removed_members .unwrap(); } } + +fn reverse_legacy_port_fixture() -> (Project, Id, Id, Id) { + let mut p = Project::empty(); + let oa_owner = add(&mut p, Kind::Actor, Layer::Oa, "oa owner"); + let sa_owner = add(&mut p, Kind::SystemActor, Layer::Sa, "sa owner"); + let oa = Id::new_v4(); + let sa = Id::new_v4(); + let mut legacy = make_port(&p, oa, oa_owner, "oa port", PortContract::Legacy).unwrap(); + *legacy.shared_mut().unwrap() = SharedBody::Port { + contract: PortContract::Legacy, + legacy: Some(LegacyPortContract { + direction: PortDirection::InOut, + interface: None, + }), + }; + p.elements.insert(oa, legacy); + let typed = make_port( + &p, + sa, + sa_owner, + "sa port", + PortContract::Component { + kind: ComponentPortKind::Flow, + orientation: ComponentPortOrientation::Out, + }, + ) + .unwrap(); + p.elements.insert(sa, typed); + let t = trace(&mut p, oa, sa); + (p, oa, sa, t) +} + +#[test] +fn reverse_existing_port_review_retains_legacy_and_roundtrips_typed_history() { + for decision in [SemanticDecision::KeepTarget, SemanticDecision::Candidate] { + let (p, oa, sa, t) = reverse_legacy_port_fixture(); + let mut r = request( + sa, + oa, + vec![(FacetSelector::Port(PortFacet::Constructor), decision)], + ); + r.transition.direction = TransitionDirection::SaToOa; + r.traces.insert(sa, t); + let expected_direction = if decision == SemanticDecision::Candidate { + let mut candidate = p.elements[&oa].clone(); + let Some(SharedBody::Port { + legacy: Some(old), .. + }) = candidate.shared_mut() + else { + panic!() + }; + old.direction = PortDirection::In; + r.candidates.push(SemanticRecord::Element(candidate)); + PortDirection::In + } else { + PortDirection::InOut + }; + let q = applied(&p, r); + assert_eq!(q.elements[&sa], p.elements[&sa]); + assert_eq!(q.elements.len(), p.elements.len()); + assert!(matches!(q.elements[&oa].shared(), Some(SharedBody::Port { + contract: PortContract::Legacy, legacy: Some(LegacyPortContract { direction, interface: None }), + }) if *direction == expected_direction)); + let baseline = q.traces[&t] + .baseline + .as_ref() + .unwrap() + .semantic + .as_ref() + .unwrap(); + validate_semantic_baseline(baseline).unwrap(); + assert!(baseline.facets.iter().any(|f| matches!( + f, + FacetReview::Port(PortReview::Constructor(Reviewed { + oa: Observed::Value(PortConstructor::Legacy), + sa: Observed::Value(PortConstructor::Component), + })) + ))); + assert_eq!(language::parse(&language::format(&q)).unwrap(), q); + assert_eq!( + persistence::decode(&serde_json::to_vec(&q).unwrap()).unwrap(), + q + ); + } +} + +#[test] +fn reverse_existing_port_review_rejects_typed_oa_proposals_without_mutation() { + for decision in [SemanticDecision::TakeSource, SemanticDecision::Candidate] { + let (p, oa, sa, _) = reverse_legacy_port_fixture(); + let snapshot = p.clone(); + let mut r = request( + sa, + oa, + vec![(FacetSelector::Port(PortFacet::Constructor), decision)], + ); + r.transition.direction = TransitionDirection::SaToOa; + if decision == SemanticDecision::Candidate { + let mut candidate = p.elements[&oa].clone(); + *candidate.shared_mut().unwrap() = p.elements[&sa].shared().unwrap().clone(); + r.candidates.push(SemanticRecord::Element(candidate)); + } + let plan = plan_semantic_transition(&p, r).unwrap(); + assert!(!plan.conflicts.is_empty()); + assert!( + plan.conflicts + .iter() + .all(|c| !c.contains("outside selected transition")), + "Must reach structural validation: {:?}", + plan.conflicts + ); + assert!(prepare_semantic_transition(&p, &plan).is_err()); + assert_eq!(p, snapshot); + } +} + +#[test] +fn reverse_new_port_counterpart_remains_an_explicit_sa_only_omission() { + let (p, _, sa, _) = reverse_legacy_port_fixture(); + let mut r = TransitionRequest::new(TransitionDirection::SaToOa, vec![sa]); + r.mappings.push(TransitionMapping { + source: sa, + target: None, + target_kind: Some(Kind::Port), + }); + let plan = plan_transition(&p, &r).unwrap(); + assert!(plan.conflicts.is_empty(), "{:?}", plan.conflicts); + assert!(plan.changes.is_empty()); + assert!(plan.warnings.iter().any(|w| w.contains("SA-only omission"))); + assert_eq!(apply_transition(&p, &plan).unwrap(), p); +} diff --git a/harness/release-constructor-history-fix.md b/harness/release-constructor-history-fix.md index bac77a9c..b1839368 100644 --- a/harness/release-constructor-history-fix.md +++ b/harness/release-constructor-history-fix.md @@ -26,3 +26,22 @@ cargo run --offline --manifest-path C:/Users/sorok/AppData/Local/Temp/capella2-c ``` 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. + + +## Separate follow-up: review an existing OA Legacy port + +The shared transition planner previously removed every typed SA port before resolving explicit existing-target mappings. Consequently an existing trace could not Keep or supply a Legacy Candidate in SA-to-OA semantic review: it failed with `Review source is outside selected transition mappings`. Two focused regressions reproduced that boundary on the committed keyed correction (`existing-port-red.log`). + +The language contract permits same-kind Port traces and reviewed updates from either layer, while retaining OA Legacy-only compatibility. The planner now retains only explicitly mapped SA ports whose selected existing OA target is Legacy. It does not synthesize a target, change a port contract, or un-omit dependent SA implementation records. Structural validation still rejects typed OA Take/Candidate proposals. Automatic/new-counterpart omissions remain unchanged. + +Three regressions exercise Keep and Candidate retaining Legacy, unchanged SA source, typed history source/JSON round trips, typed OA proposals failing without mutation, and an explicit new Port counterpart remaining an SA-only omission. A separate read-only review found no scope/invariant defect. Focused red-to-green result: all three pass (`existing-port-green.log`). This change uses the common planner consumed by desktop and REST; it adds no endpoint or transport-specific behavior. + +```powershell +$env:CARGO_TARGET_DIR='D:/CodexBuilds/Capella2-scenario-target' +cargo test --locked --offline -p oa-core --test r3_semantic reverse_ -- --nocapture +cargo test --locked --offline -p oa-core +cargo clippy --locked --offline -p oa-core --all-targets -- -D warnings +cargo fmt --all -- --check +``` + +All 359 Windows core tests pass, including all 34 semantic tests and existing portless-OA transition coverage. Strict core all-target Clippy and workspace formatting pass. Logs: `existing-port-core-test.log` and `existing-port-core-clippy.log` in the dedicated target. REST preview uses the corrected core planner at `oa-server/src/routes.rs` and commit uses the shared core prepare path; those handlers were inspected, not changed. GUI constructor tests and actual API/native acceptance remain with root integration; no native input or desktop code was changed here.