From 58eef41c7e93cc4ab3f8b8b422574c2bc8109a45 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 7 Jul 2026 20:18:29 -0400 Subject: [PATCH] Phase D follow-up: anchor-site liveness and containment, all object kinds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User-review findings on 9b5339f — the Phase-D site-set unification was incomplete in exactly two consumers that still collapsed anchor_sites() to events: - High: the mint precondition validated only TimeAnchor::Event targets, so CreateRepeatStructure with start naming a missing REGION (or a volta span a missing MEASURE) minted a dangling repeat straight past CrossCuttingRefsResolve. Fixed: anchor_object_refs (events + measures + regions; wall-clock references nothing) drives the precondition — deterministic across reduce()/reduce_onto(), since the base seed registers regions and measures in `objects`. Regression covers a missing region in start (base-free), a ghost measure inside a volta span (graph-aware), and the positive measure/region-anchored mint with invariants green. The referent INDEX stays event-only by design (the rule table repairs event tombstones — the spanner discipline). - Medium: editor barrier containment derived only from event locations, so a repeat anchored solely to a protected region carried a default context and bypassed a region-scoped barrier. Fixed: repeat_context walks all anchor objects in anchor_sites order — event/measure sites bind (region, staff instance) via event_location/measure_location, a bare region anchor binds the region — used by both Create and Delete subject arms. Regression: a region-scoped barrier fires for a region-anchored repeat create and stays quiet for another region. P13-D3 filed (spec/PASS13_CANDIDATES.md + ops DECISIONS): the SPANNER family has the same mint-time shape (CrossCuttingValue::endpoints() is events-only while anchor_target_exists checks all three kinds) plus the non-event-referent-tombstone gap — pre-existing, ratified-as-implemented; a catalog-semantics decision, not a Phase-D fix. Full gate: fmt, clippy -D warnings, rustdoc -D warnings, 30 workspace suites, conformance scale 1 (8/8). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NEs4aYiu8MXjdYdMxw8PTd --- crates/epiphany-editor-core/src/barriers.rs | 74 +++++--- crates/epiphany-editor-core/src/lib.rs | 56 ++++++ crates/epiphany-ops/DECISIONS.md | 27 +++ crates/epiphany-ops/src/reduce.rs | 178 ++++++++++++++++++-- spec/PASS13_CANDIDATES.md | 1 + 5 files changed, 301 insertions(+), 35 deletions(-) diff --git a/crates/epiphany-editor-core/src/barriers.rs b/crates/epiphany-editor-core/src/barriers.rs index 3968863..ea19d67 100644 --- a/crates/epiphany-editor-core/src/barriers.rs +++ b/crates/epiphany-editor-core/src/barriers.rs @@ -215,19 +215,47 @@ fn structure_context(score: &Score, endpoints: &[TypedObjectId]) -> EditContext ctx(location.map(|(r, _)| r), location.map(|(_, si)| si)) } -/// The event references among a repeat structure's anchor sites -/// ([`epiphany_core::RepeatStructure::anchor_sites`] — start/end, jump -/// targets, volta spans; the same site set reduction's re-anchoring rule -/// covers). -fn repeat_event_refs(repeat: &epiphany_core::RepeatStructure) -> Vec { - repeat - .anchor_sites() - .into_iter() - .filter_map(|anchor| match anchor { - epiphany_core::TimeAnchor::Event { id, .. } => Some(TypedObjectId::Event(*id)), - _ => None, +/// The containment of a live measure: the staff instance whose measure list +/// carries it, with its region. +fn measure_location( + score: &Score, + measure: epiphany_core::MeasureId, +) -> Option<(RegionId, StaffInstanceId)> { + score.canvas.regions.iter().find_map(|region| { + region.staff_instances().iter().find_map(|si| { + si.measures + .iter() + .any(|m| m.id == measure) + .then_some((region.id, si.id)) }) - .collect() + }) +} + +/// The containment context a repeat structure's anchor sites bind: the first +/// object-referencing site (in [`epiphany_core::RepeatStructure::anchor_sites`] +/// order — start, end, jump targets, volta spans) that resolves. An event or +/// measure site binds its (region, staff instance); a bare region anchor +/// binds the region alone. Region- and measure-anchored repeats must derive +/// real containment here — an event-only walk would let a repeat anchored +/// solely to a protected region bypass a region-scoped barrier. +fn repeat_context(score: &Score, repeat: &epiphany_core::RepeatStructure) -> EditContext { + for site in repeat.anchor_sites() { + match site { + epiphany_core::TimeAnchor::Event { id, .. } => { + if let Some((region, instance)) = event_location(score, *id) { + return ctx(Some(region), Some(instance)); + } + } + epiphany_core::TimeAnchor::Measure { id, .. } => { + if let Some((region, instance)) = measure_location(score, *id) { + return ctx(Some(region), Some(instance)); + } + } + epiphany_core::TimeAnchor::Region { id, .. } => return ctx(Some(*id), None), + epiphany_core::TimeAnchor::WallClock { .. } => {} + } + } + ctx(None, None) } /// The endpoints of a cross-cutting structure already in the graph, by id. @@ -272,13 +300,6 @@ fn graph_structure_endpoints(score: &Score, structure: &TypedObjectId) -> Vec score - .cross_cutting - .repeats - .iter() - .find(|r| r.id == *id) - .map(repeat_event_refs) - .unwrap_or_default(), _ => Vec::new(), } } @@ -434,14 +455,17 @@ pub(crate) fn subjects_of(kind: &OperationKind, score: &Score) -> BarrierSubject } OperationKind::CreateRepeatStructure(op) => one( TypedObjectId::RepeatStructure(op.repeat_structure_id()), - structure_context(score, &repeat_event_refs(&op.repeat)), + repeat_context(score, &op.repeat), ), OperationKind::DeleteRepeatStructure(op) => { - let sid = TypedObjectId::RepeatStructure(op.repeat); - one( - sid, - structure_context(score, &graph_structure_endpoints(score, &sid)), - ) + let context = score + .cross_cutting + .repeats + .iter() + .find(|r| r.id == op.repeat) + .map(|r| repeat_context(score, r)) + .unwrap_or_else(|| ctx(None, None)); + one(TypedObjectId::RepeatStructure(op.repeat), context) } OperationKind::Registered(..) => BarrierSubjects::Unknown, } diff --git a/crates/epiphany-editor-core/src/lib.rs b/crates/epiphany-editor-core/src/lib.rs index c0aa089..49258c3 100644 --- a/crates/epiphany-editor-core/src/lib.rs +++ b/crates/epiphany-editor-core/src/lib.rs @@ -5841,6 +5841,62 @@ mod tests { ); } + #[test] + fn a_region_scoped_barrier_gates_a_region_anchored_repeat() { + // A repeat anchored ONLY to a region derives that region as its + // containment (repeat_context walks all anchor objects, not just + // events) — a region-scoped barrier over it must fire, and one over + // another region must not. (An event-only walk left region-anchored + // repeats with a default context, silently bypassing the scope.) + let scoped = |region| ActiveExtension { + extension: ExtensionRef(0xE9), + barriers: vec![EditBarrier { + scope: BarrierScope::Region(region), + affected_object_kinds: vec![], + prohibited_operation_kinds: vec![OperationKindTag::CreateRepeatStructure], + condition: BarrierCondition::Always, + }], + }; + let create_in = |session: &EditorSession| { + let here = session.score().canvas.regions[0].id; + let anchor = |edge| epiphany_core::TimeAnchor::Region { + id: here, + edge, + offset: epiphany_core::AnchorOffset::Zero, + }; + OperationKind::CreateRepeatStructure(epiphany_ops::CreateRepeatStructureOp { + repeat: epiphany_core::RepeatStructure { + id: epiphany_core::RepeatStructureId::new(epiphany_core::ReplicaId(0xEE), 1), + start: anchor(epiphany_core::RegionEdge::Start), + end: anchor(epiphany_core::RegionEdge::End), + kind: epiphany_core::RepeatKind::SimpleRepeat { count: 2 }, + voltas: Vec::new(), + }, + }) + }; + + let mut session = open_plain(7); + let elsewhere = RegionId::from_raw(u128::MAX); + session.set_active_extensions(vec![scoped(elsewhere)]); + let create = create_in(&session); + session + .apply(create) + .expect("a barrier over another region does not bind here"); + + let mut session = open_plain(7); + let here = session.score().canvas.regions[0].id; + session.set_active_extensions(vec![scoped(here)]); + let create = create_in(&session); + assert_eq!( + session.apply(create), + Err(EditorError::BarrierProhibited { + extension: ExtensionRef(0xE9), + operation: OperationKindTag::CreateRepeatStructure, + }), + "the protected region's own barrier fires for a region-anchored repeat" + ); + } + #[test] fn an_unsafe_edit_crosses_the_barrier_and_records_the_tombstone_obligation() { let mut session = open_plain(7); diff --git a/crates/epiphany-ops/DECISIONS.md b/crates/epiphany-ops/DECISIONS.md index 8d07e48..8044a20 100644 --- a/crates/epiphany-ops/DECISIONS.md +++ b/crates/epiphany-ops/DECISIONS.md @@ -1078,3 +1078,30 @@ stamps the same fixed minor as always) — pre-existing for the Phase-3 kinds, now also true of the delete; a reader hitting the unknown discriminant cannot attribute the failure to version skew from the stamp alone. A bundle-writer design item for the next bundle tranche. + +### Follow-up (user review): the anchor-site set, consumed WHOLLY + +Post-commit review found the Phase-D unification incomplete in exactly two +places that still collapsed `anchor_sites()` to events: (1) the mint +precondition validated only `TimeAnchor::Event` targets, so a create whose +start named a missing REGION (or a volta span a missing MEASURE) minted a +dangling repeat straight past `CrossCuttingRefsResolve` — fixed with +`anchor_object_refs` (events + measures + regions; wall-clock references +nothing), deterministic across both reduction modes since the base seed +registers regions and measures in `objects`; and (2) the editor barrier +containment derived only from event locations, so a repeat anchored solely +to a protected region carried a default context and bypassed a +region-scoped barrier — fixed with `repeat_context` (first +object-referencing site in anchor order binds: event/measure → +(region, instance); bare region anchor → the region). Both +regression-locked. The referent INDEX stays event-only by design (the rule +table repairs event tombstones; the spanner discipline). + +**P13-D3 filed:** `CreateCrossCutting` has the same mint-time shape for +SPANNERS — `CrossCuttingValue::endpoints()` filters to events, so a spanner +anchored to a missing region/measure mints dangling past the invariant +(`anchor_target_exists` checks spanner anchors at all three kinds), and +non-event referent tombstones (a `DeleteRegion` under a region-anchored +spanner or repeat) re-anchor nothing. Pre-existing, ratified-as-implemented +("every referenced endpoint is live" reads events-only in the code); +changing it is a catalog-semantics decision, not a Phase-D fix. diff --git a/crates/epiphany-ops/src/reduce.rs b/crates/epiphany-ops/src/reduce.rs index 7104e40..5c78d9b 100644 --- a/crates/epiphany-ops/src/reduce.rs +++ b/crates/epiphany-ops/src/reduce.rs @@ -1107,6 +1107,25 @@ fn reason_for_rank(rank: u8) -> ReanchorReason { } } +/// The OBJECT references among a set of [`TimeAnchor`]s — events, measures, +/// and regions (a wall-clock anchor references nothing). The mint-time +/// liveness set: every one of these must be `Live` for a mint to leave the +/// graph satisfying the reference-resolution invariants +/// (`anchor_target_exists` checks exactly these three kinds). Both reduction +/// modes agree: the base seed registers regions and measures in `objects`, +/// and base-free reductions simply have fewer live objects. +fn anchor_object_refs<'a>(anchors: impl IntoIterator) -> Vec { + anchors + .into_iter() + .filter_map(|anchor| match anchor { + TimeAnchor::Event { id, .. } => Some(TypedObjectId::Event(*id)), + TimeAnchor::Measure { id, .. } => Some(TypedObjectId::Measure(*id)), + TimeAnchor::Region { id, .. } => Some(TypedObjectId::Region(*id)), + TimeAnchor::WallClock { .. } => None, + }) + .collect() +} + /// The event references among a set of [`TimeAnchor`]s (the referent-index /// entries a tombstone must repair). Non-event anchors contribute nothing. fn anchor_event_refs<'a>(anchors: impl IntoIterator) -> Vec { @@ -3210,13 +3229,14 @@ impl<'a> Reducer<'a> { } None => {} } - // Every event-referencing anchor site must resolve live — start/end, - // the kind's jump targets, each volta's span (operation_catalog - // §"Repeat Structures": the mint must leave the graph satisfying the - // reference-resolution invariants). - let endpoints = anchor_event_refs(op.repeat.anchor_sites()); - for e in &endpoints { - if !matches!(self.objects.get(e), Some(ObjectState::Live)) { + // Every OBJECT-referencing anchor site must resolve live — start/end, + // the kind's jump targets, each volta's span; events, measures, AND + // regions (operation_catalog §"Repeat Structures": the mint must + // leave the graph satisfying the reference-resolution invariants, + // which check all three anchor kinds — an event-only check would + // mint a dangling repeat off a missing region/measure anchor). + for referenced in anchor_object_refs(op.repeat.anchor_sites()) { + if !matches!(self.objects.get(&referenced), Some(ObjectState::Live)) { return OperationEffect::NoOp { reason: NoOpReason::PreconditionFailedUnderReduction { reason: PreconditionFailureReason::TargetMissing, @@ -3230,9 +3250,12 @@ impl<'a> Reducer<'a> { self.objects.insert(sid, ObjectState::Live); self.minted_by.insert(sid, env.id); self.note_minted(env, sid); - // Register into the referent index so event tombstones repair the - // structure per the rule table (no write chain: there is no - // ModifyRepeatStructure at this revision). + // Register the EVENT refs into the referent index so event tombstones + // repair the structure per the rule table (no write chain: there is + // no ModifyRepeatStructure at this revision). Measure/region anchors + // stay unindexed, as for spanners — non-event referent tombstones + // are the filed P13-D3 class. + let endpoints = anchor_event_refs(op.repeat.anchor_sites()); if !endpoints.is_empty() { self.structures.insert(sid, endpoints); } @@ -8066,6 +8089,141 @@ mod tests { assert!(epiphany_core::check_invariants(&result.score).is_empty()); } + #[test] + fn create_repeat_structure_preconditions_non_event_anchor_targets() { + // The all-anchor-sites-live precondition covers OBJECT references of + // every kind — a missing REGION or MEASURE target refuses the mint + // exactly like a missing event (an event-only check minted a + // dangling repeat straight past CrossCuttingRefsResolve). + use epiphany_core::generators::valid_score_rich; + let rid = epiphany_core::RepeatStructureId::new(ReplicaId(1), 1); + + // Base-free: live event anchors, but start names a missing region. + let e1 = EventId::new(ReplicaId(1), 100); + let e2 = EventId::new(ReplicaId(1), 101); + let mut repeat = crate::valuegen::repeat_structure(rid, e1, e2); + repeat.start = TimeAnchor::Region { + id: RegionId::new(ReplicaId(1), 6_666), + edge: RegionEdge::Start, + offset: AnchorOffset::Zero, + }; + let mut set = OperationSet::new(); + set.accept_all(vec![ + insert(1, 0, 10, 1, 100, 0), + insert(1, 1, 11, 1, 101, 1), + prim_env( + 1, + 2, + 12, + seen_r1(1), + OperationKind::CreateRepeatStructure(CreateRepeatStructureOp { repeat }), + ), + ]); + assert!( + !set.reduce() + .objects + .contains_key(&TypedObjectId::RepeatStructure(rid)), + "a missing region anchor target refuses the mint" + ); + + // Graph-aware: a live measure + live region anchor MINTS (and the + // graph stays invariant-clean); a ghost measure in a volta span + // refuses. + let base = valid_score_rich(0x0D0D); + let (region_id, measure_id) = base + .canvas + .regions + .iter() + .find_map(|r| { + r.staff_instances() + .iter() + .find_map(|si| si.measures.first().map(|m| (r.id, m.id))) + }) + .expect("the rich fixture has a measure"); + let measure_anchor = TimeAnchor::Measure { + id: measure_id, + position: epiphany_core::MeasurePosition::Start, + offset: AnchorOffset::Zero, + }; + let region_anchor = TimeAnchor::Region { + id: region_id, + edge: RegionEdge::End, + offset: AnchorOffset::Zero, + }; + let live = epiphany_core::RepeatStructure { + id: rid, + start: measure_anchor.clone(), + end: region_anchor.clone(), + kind: epiphany_core::RepeatKind::SimpleRepeat { count: 2 }, + voltas: Vec::new(), + }; + let mut set = OperationSet::new(); + set.accept_all(vec![prim_env( + 2, + 0, + 10, + CausalContext::new(), + OperationKind::CreateRepeatStructure(CreateRepeatStructureOp { repeat: live }), + )]); + let result = set.reduce_onto(&base); + assert!( + matches!( + result + .state + .objects + .get(&TypedObjectId::RepeatStructure(rid)), + Some(ObjectState::Live) + ), + "live measure/region anchors mint" + ); + assert!(epiphany_core::check_invariants(&result.score).is_empty()); + + let mut ghost_volta = epiphany_core::RepeatStructure { + id: epiphany_core::RepeatStructureId::new(ReplicaId(1), 2), + start: measure_anchor, + end: region_anchor.clone(), + kind: epiphany_core::RepeatKind::Volta, + voltas: vec![epiphany_core::Volta { + endings: vec![1], + start: region_anchor, + end: TimeAnchor::Measure { + id: epiphany_core::MeasureId::new(ReplicaId(1), 6_666), + position: epiphany_core::MeasurePosition::Start, + offset: AnchorOffset::Zero, + }, + }], + }; + let ghost_id = ghost_volta.id; + ghost_volta.voltas[0].endings = vec![1]; + let mut set = OperationSet::new(); + set.accept_all(vec![prim_env( + 2, + 0, + 10, + CausalContext::new(), + OperationKind::CreateRepeatStructure(CreateRepeatStructureOp { + repeat: ghost_volta, + }), + )]); + let result = set.reduce_onto(&base); + assert!( + !result + .state + .objects + .contains_key(&TypedObjectId::RepeatStructure(ghost_id)), + "a missing measure target inside a volta span refuses the mint" + ); + assert!( + !result + .score + .cross_cutting + .repeats + .iter() + .any(|r| r.id == ghost_id), + "nothing dangling reaches the graph" + ); + } + #[test] fn deleting_an_event_rewires_a_dal_segno_jump_target() { // The kind's jump targets are anchor SITES like any other: a dead diff --git a/spec/PASS13_CANDIDATES.md b/spec/PASS13_CANDIDATES.md index cdc8fa3..70d8541 100644 --- a/spec/PASS13_CANDIDATES.md +++ b/spec/PASS13_CANDIDATES.md @@ -10,3 +10,4 @@ this file is the index, not the analysis. | P13-K1 | The K3 verdict for a system pitch introduced by a ModifyEvent replacement value differs across a snapshot cut (in-session `TargetMissing` vs post-snapshot `SystemDerivedContentImmutable`); really "may ModifyEvent introduce never-minted pitch ids" | `crates/epiphany-ops/DECISIONS.md` (Pass-12 G-pass code tranche) | open | | P13-D1 | Undo-driven event tombstones run graph-side re-anchor/cascade but never ledger-side `reanchor_for_tombstone`: structures leave the graph while staying `Live`, no `RepairRecord` — Ch6's same-step recording MUST is unmet for undo-driven tombstones (pre-existing class: slurs/spanners; repeats now too) | `crates/epiphany-ops/DECISIONS.md` (Schema major 2, Phase D) | open | | P13-D2 | Cue-cascade recursion re-anchors against the triggering event before its tombstone lands in `objects`: a structure anchored on {X, cue-of-X} can record `Reanchored{to: X}` then `CascadeDeleted` in one effect (contradictory repair trail; plausible by code trace, unexecuted) | `crates/epiphany-ops/DECISIONS.md` (Schema major 2, Phase D) | open | +| P13-D3 | `CreateCrossCutting` validates only event endpoints (`CrossCuttingValue::endpoints()`), so a SPANNER anchored to a missing region/measure mints dangling past `anchor_target_exists`; and non-event referent tombstones (`DeleteRegion` under a region-anchored spanner/repeat) re-anchor nothing — "every referenced endpoint is live" is events-only as implemented | `crates/epiphany-ops/DECISIONS.md` (Phase D follow-up) | open |