Agent K M2d review follow-up: harden the Group-4 score-settings ops
Address the five-finding review of M2d (1e4ab24) plus the two-finding
follow-up review; all fixes are code/test/doc only, no spec change needed
(the catalog/core-spec already classify metadata as advisory LWW).
- SetMetadata is now a true advisory LWW: it silently last-writer-wins in
canonical order and records no conflict, matching operation_catalog
§set-user-system-break. Dropped the StructuralFieldCollision, the
`last_metadata` working slot, and the `env` parameter; rewrote the
conflict test as `concurrent_differing_set_metadata_is_advisory_lww`
(no conflict, state stays clean, permutation-independent resolution).
- SetMetricGrid / SetUserPageBreak / SetUserSystemBreak share a new
`layout_region_slot` precondition backed by a `staff_based_regions`
index: the target must be live and staff-based (FreeGraphic regions have
neither a metric-grid nor a break slot). The index is read from base-free
state, so reduce() and reduce_onto() reach the same verdict for missing,
tombstoned, and FreeGraphic regions.
- SetMetricGrid now rejects a grid whose meter_sequence names an undeclared
time signature, rather than installing an invariant-violating grid.
- User breaks materialize under the canonical LWW key: `apply_break_lww`
drops any existing anchor resolving to the same position before adding,
so the graph break list stays in lockstep with the resolved-position
ledger map (shared `resolved_anchor_position`). Applied to page and
system breaks alike.
- Coverage: SetMetadata/SetMetricGrid/CreateVoice/DeleteVoice added to the
tag-distinctness test; layout_stub `gen_operation_kind_tag` extended to
every normative tag; the MaterializedState decode test populates
page_breaks; four direct regression tests pin each fixed bug; the stale
SetMetadata/score_metadata doc comments now say advisory LWW.
Gates: build/fmt/clippy -D warnings clean; cargo test --workspace green
(533); conformance_suite scale 1 passes. Stages only core/ops/testkit; the
unrelated Agent-I working tree is left untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
1e4ab24779
commit
d93baac3ba
|
|
@ -583,6 +583,15 @@ impl RegionContent {
|
|||
}
|
||||
}
|
||||
|
||||
/// Mutable staff-based content, if this region is staff-based or hybrid.
|
||||
pub fn staff_based_mut(&mut self) -> Option<&mut StaffBasedContent> {
|
||||
match self {
|
||||
RegionContent::StaffBased(c) => Some(c),
|
||||
RegionContent::Hybrid { staves, .. } => Some(staves),
|
||||
RegionContent::FreeGraphic(_) => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Mutable access to the staff instances, if this content has any (used by
|
||||
/// editing and by the invariant shrinker in [`crate::generators`]).
|
||||
pub fn staff_instances_mut(&mut self) -> Option<&mut Vec<StaffInstance>> {
|
||||
|
|
|
|||
|
|
@ -626,19 +626,27 @@ pub struct SetUserSystemBreakOp {
|
|||
pub present: bool,
|
||||
}
|
||||
|
||||
/// The musical position a [`TimeAnchor`] resolves to for user-break LWW
|
||||
/// bucketing. A region-relative musical offset resolves to that offset; any
|
||||
/// other anchor shape resolves to the region origin (the break still applies to
|
||||
/// the region, the prototype LWW key is coarse — see `DECISIONS.md`). The graph
|
||||
/// reducer keys break anchors by this same position, so two anchors resolving
|
||||
/// here to one position occupy a single LWW slot.
|
||||
pub(crate) fn resolved_anchor_position(anchor: &TimeAnchor) -> MusicalPosition {
|
||||
match anchor {
|
||||
TimeAnchor::Region {
|
||||
offset: epiphany_core::AnchorOffset::Musical(d),
|
||||
..
|
||||
} => MusicalPosition(d.0.clone()),
|
||||
_ => MusicalPosition::origin(),
|
||||
}
|
||||
}
|
||||
|
||||
impl SetUserSystemBreakOp {
|
||||
/// The anchor's resolved musical position — the canonical LWW bucketing key.
|
||||
/// A region-relative musical offset resolves to that offset; any other anchor
|
||||
/// shape resolves to the region origin (the break still applies to the
|
||||
/// region, the prototype LWW key is coarse — see `DECISIONS.md`).
|
||||
/// The anchor's resolved musical position — the canonical LWW bucketing key
|
||||
/// (see [`resolved_anchor_position`]).
|
||||
pub fn resolved_position(&self) -> MusicalPosition {
|
||||
match &self.anchor {
|
||||
TimeAnchor::Region {
|
||||
offset: epiphany_core::AnchorOffset::Musical(d),
|
||||
..
|
||||
} => MusicalPosition(d.0.clone()),
|
||||
_ => MusicalPosition::origin(),
|
||||
}
|
||||
resolved_anchor_position(&self.anchor)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -976,8 +984,9 @@ impl CanonicalEncode for DeleteVoiceOp {
|
|||
// --- Group 4 (M2d): score settings (Chapter 6 §6.10). LWW field-overwrite. ---
|
||||
|
||||
/// Overwrite the score metadata (Chapter 6 §6.10 SetMetadata). Carries the full
|
||||
/// [`ScoreMetadata`] (v1); the score-singleton field-overwrite is last-writer-wins
|
||||
/// (concurrent differing ⇒ structural-field-collision).
|
||||
/// [`ScoreMetadata`] (v1); the score-singleton field-overwrite is *advisory*
|
||||
/// last-writer-wins — the latest write in canonical order silently wins and no
|
||||
/// conflict is recorded (operation_catalog §set-user-system-break "LWW advisory").
|
||||
#[derive(Clone, PartialEq, Eq, Debug)]
|
||||
pub struct SetMetadataOp {
|
||||
pub metadata: ScoreMetadata,
|
||||
|
|
@ -1025,15 +1034,9 @@ pub struct SetUserPageBreakOp {
|
|||
|
||||
impl SetUserPageBreakOp {
|
||||
/// The anchor's resolved musical position — the canonical LWW bucketing key
|
||||
/// (see [`SetUserSystemBreakOp::resolved_position`]).
|
||||
/// (see [`resolved_anchor_position`]).
|
||||
pub fn resolved_position(&self) -> MusicalPosition {
|
||||
match &self.anchor {
|
||||
TimeAnchor::Region {
|
||||
offset: epiphany_core::AnchorOffset::Musical(d),
|
||||
..
|
||||
} => MusicalPosition(d.0.clone()),
|
||||
_ => MusicalPosition::origin(),
|
||||
}
|
||||
resolved_anchor_position(&self.anchor)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1098,6 +1101,10 @@ mod tests {
|
|||
OperationKindTag::InsertIdentifiedPitch,
|
||||
OperationKindTag::DeleteIdentifiedPitch,
|
||||
OperationKindTag::ModifyIdentifiedPitch,
|
||||
OperationKindTag::CreateVoice,
|
||||
OperationKindTag::DeleteVoice,
|
||||
OperationKindTag::SetMetadata,
|
||||
OperationKindTag::SetMetricGrid,
|
||||
];
|
||||
let encoded: std::collections::BTreeSet<_> = tags
|
||||
.iter()
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ use std::collections::{BTreeMap, BTreeSet};
|
|||
use epiphany_core::{
|
||||
derive_promoted_voice_id, AnchorOffset, CanonicalValue, Event, EventDuration, EventId,
|
||||
EventPosition, MetricGrid, MusicalDuration, MusicalPosition, OperationId, Pitch, PitchId,
|
||||
PitchSpelling, RegionEdge, RegionId, RegionTimeModel, Score, ScoreMetadata, SpellingAttachment,
|
||||
PitchSpelling, RegionEdge, RegionId, RegionTimeModel, Score, SpellingAttachment,
|
||||
SpellingDirective, SpellingScope, SpellingSource, StaffInstance, StaffInstanceId, TimeAnchor,
|
||||
TransactionId, TypedObjectId, Voice, VoiceId, VoiceOrigin,
|
||||
};
|
||||
|
|
@ -341,9 +341,10 @@ struct Reducer<'a> {
|
|||
// LWW working state for ModifyCrossCutting (Group 2), mirroring the leaf-field
|
||||
// modify maps above: last modifier + value it wrote, keyed by structure id.
|
||||
last_cross_cutting_modify: BTreeMap<TypedObjectId, (OperationId, CrossCuttingValue)>,
|
||||
// LWW working state for the Group-4 (M2d) score-settings overwrites: the last
|
||||
// writer and the value it wrote (the resolved value lives in the graph).
|
||||
last_metadata: Option<(OperationId, ScoreMetadata)>,
|
||||
// LWW working state for SetMetricGrid (Group 4, M2d): the last writer and the
|
||||
// grid it wrote, used to detect concurrent *differing* grids (the resolved
|
||||
// value lives in the graph). SetMetadata is advisory LWW — no working state,
|
||||
// no conflict — so the latest write in canonical order silently wins.
|
||||
last_metric_grid: BTreeMap<RegionId, (OperationId, Option<MetricGrid>)>,
|
||||
structures: BTreeMap<TypedObjectId, Vec<TypedObjectId>>,
|
||||
// Live child sets for the structural-container empty-only delete (Group 3):
|
||||
|
|
@ -351,6 +352,12 @@ struct Reducer<'a> {
|
|||
// voice's live events are read from `voice_occupancy`.)
|
||||
region_instances: BTreeMap<RegionId, BTreeSet<StaffInstanceId>>,
|
||||
instance_voices: BTreeMap<StaffInstanceId, BTreeSet<VoiceId>>,
|
||||
// Regions whose content carries a staff-based slot (staff-based or hybrid),
|
||||
// and so can hold a metric grid or user break. FreeGraphic regions cannot;
|
||||
// tracking this in the base-free reducer lets SetMetricGrid / SetUserPageBreak
|
||||
// / SetUserSystemBreak reach the same precondition verdict with or without a
|
||||
// graph, so reduce() and reduce_onto() never disagree on those ops.
|
||||
staff_based_regions: BTreeSet<RegionId>,
|
||||
migrated_regions: BTreeSet<RegionId>,
|
||||
region_migrator: BTreeMap<RegionId, OperationId>,
|
||||
descriptors: BTreeMap<TransactionId, OperationId>,
|
||||
|
|
@ -375,11 +382,11 @@ struct WorkingSnapshot {
|
|||
last_event_modify: BTreeMap<EventId, (OperationId, Event)>,
|
||||
last_pitch_modify: BTreeMap<PitchId, (OperationId, Pitch)>,
|
||||
last_cross_cutting_modify: BTreeMap<TypedObjectId, (OperationId, CrossCuttingValue)>,
|
||||
last_metadata: Option<(OperationId, ScoreMetadata)>,
|
||||
last_metric_grid: BTreeMap<RegionId, (OperationId, Option<MetricGrid>)>,
|
||||
structures: BTreeMap<TypedObjectId, Vec<TypedObjectId>>,
|
||||
region_instances: BTreeMap<RegionId, BTreeSet<StaffInstanceId>>,
|
||||
instance_voices: BTreeMap<StaffInstanceId, BTreeSet<VoiceId>>,
|
||||
staff_based_regions: BTreeSet<RegionId>,
|
||||
migrated_regions: BTreeSet<RegionId>,
|
||||
region_migrator: BTreeMap<RegionId, OperationId>,
|
||||
descriptors: BTreeMap<TransactionId, OperationId>,
|
||||
|
|
@ -387,6 +394,19 @@ struct WorkingSnapshot {
|
|||
graph: Option<Score>,
|
||||
}
|
||||
|
||||
/// Apply a user break to a region's break list under the canonical LWW key — the
|
||||
/// anchor's resolved musical position. Any existing anchor resolving to that same
|
||||
/// position is dropped first, so two anchors at one position never both persist,
|
||||
/// then the new anchor is pushed iff the break is present. The graph break list
|
||||
/// then matches the resolved-position-keyed ledger map.
|
||||
fn apply_break_lww(breaks: &mut Vec<TimeAnchor>, anchor: &TimeAnchor, present: bool) {
|
||||
let resolved = crate::payload::resolved_anchor_position(anchor);
|
||||
breaks.retain(|existing| crate::payload::resolved_anchor_position(existing) != resolved);
|
||||
if present {
|
||||
breaks.push(anchor.clone());
|
||||
}
|
||||
}
|
||||
|
||||
fn intervals_overlap(
|
||||
a_position: &MusicalPosition,
|
||||
a_duration: &MusicalDuration,
|
||||
|
|
@ -453,11 +473,11 @@ impl<'a> Reducer<'a> {
|
|||
last_event_modify: BTreeMap::new(),
|
||||
last_pitch_modify: BTreeMap::new(),
|
||||
last_cross_cutting_modify: BTreeMap::new(),
|
||||
last_metadata: None,
|
||||
last_metric_grid: BTreeMap::new(),
|
||||
structures: BTreeMap::new(),
|
||||
region_instances: BTreeMap::new(),
|
||||
instance_voices: BTreeMap::new(),
|
||||
staff_based_regions: BTreeSet::new(),
|
||||
migrated_regions: BTreeSet::new(),
|
||||
region_migrator: BTreeMap::new(),
|
||||
descriptors: BTreeMap::new(),
|
||||
|
|
@ -518,6 +538,9 @@ impl<'a> Reducer<'a> {
|
|||
for region in &score.canvas.regions {
|
||||
self.objects
|
||||
.insert(TypedObjectId::Region(region.id), ObjectState::Live);
|
||||
if region.content.staff_based().is_some() {
|
||||
self.staff_based_regions.insert(region.id);
|
||||
}
|
||||
let instance_set = self.region_instances.entry(region.id).or_default();
|
||||
for instance in region.staff_instances() {
|
||||
instance_set.insert(instance.id);
|
||||
|
|
@ -1250,7 +1273,7 @@ impl<'a> Reducer<'a> {
|
|||
OperationKind::DeleteStaffInstance(op) => self.delete_staff_instance(env, op),
|
||||
OperationKind::CreateVoice(op) => self.create_voice(env, op),
|
||||
OperationKind::DeleteVoice(op) => self.delete_voice(env, op),
|
||||
OperationKind::SetMetadata(op) => self.set_metadata(env, op),
|
||||
OperationKind::SetMetadata(op) => self.set_metadata(op),
|
||||
OperationKind::SetMetricGrid(op) => self.set_metric_grid(env, op),
|
||||
OperationKind::SetUserPageBreak(op) => self.set_user_page_break(op),
|
||||
},
|
||||
|
|
@ -1265,43 +1288,14 @@ impl<'a> Reducer<'a> {
|
|||
&mut self,
|
||||
op: &crate::payload::SetUserSystemBreakOp,
|
||||
) -> OperationEffect {
|
||||
if let Some(effect) = self.layout_region_slot(op.region) {
|
||||
return effect;
|
||||
}
|
||||
if let Some(score) = self.graph.as_mut() {
|
||||
let Some(region) = score
|
||||
.canvas
|
||||
.regions
|
||||
.iter_mut()
|
||||
.find(|region| region.id == op.region)
|
||||
else {
|
||||
return OperationEffect::NoOp {
|
||||
reason: NoOpReason::PreconditionFailedUnderReduction {
|
||||
reason: PreconditionFailureReason::TargetMissing,
|
||||
},
|
||||
};
|
||||
};
|
||||
let breaks = match &mut region.content {
|
||||
epiphany_core::RegionContent::StaffBased(content) => {
|
||||
&mut content.user_system_breaks
|
||||
if let Some(region) = score.canvas.regions.iter_mut().find(|r| r.id == op.region) {
|
||||
if let Some(content) = region.content.staff_based_mut() {
|
||||
apply_break_lww(&mut content.user_system_breaks, &op.anchor, op.present);
|
||||
}
|
||||
epiphany_core::RegionContent::Hybrid { staves, .. } => {
|
||||
&mut staves.user_system_breaks
|
||||
}
|
||||
epiphany_core::RegionContent::FreeGraphic(_) => {
|
||||
return OperationEffect::NoOp {
|
||||
reason: NoOpReason::PreconditionFailedUnderReduction {
|
||||
reason: PreconditionFailureReason::TargetMissing,
|
||||
},
|
||||
}
|
||||
}
|
||||
};
|
||||
// The value-typed payload (v1) carries the full TimeAnchor, so the
|
||||
// graph break is the anchor itself rather than a reconstructed one.
|
||||
let anchor = op.anchor.clone();
|
||||
if op.present {
|
||||
if !breaks.contains(&anchor) {
|
||||
breaks.push(anchor);
|
||||
}
|
||||
} else {
|
||||
breaks.retain(|candidate| candidate != &anchor);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1313,40 +1307,44 @@ impl<'a> Reducer<'a> {
|
|||
|
||||
// --- Group 4 (M2d): score settings (LWW field-overwrite). --------------
|
||||
//
|
||||
// SetMetadata / SetMetricGrid mirror the modify ops: the resolved value lives
|
||||
// in the graph (reduce_onto); MaterializedState records only the effect and,
|
||||
// on a concurrent differing write, a StructuralFieldCollision. SetUserPageBreak
|
||||
// mirrors SetUserSystemBreak: it is a canonical LWW advisory (page_breaks).
|
||||
// SetMetadata is an *advisory* last-writer-wins field (operation_catalog
|
||||
// §set-user-system-break "LWW advisory"): the latest write in canonical order
|
||||
// silently wins, with no conflict — a clean concurrent metadata edit keeps the
|
||||
// state clean. SetMetricGrid is a structural field-overwrite: the resolved grid
|
||||
// lives in the graph and a concurrent differing grid records a
|
||||
// StructuralFieldCollision. SetUserPageBreak mirrors SetUserSystemBreak: a
|
||||
// canonical LWW advisory (page_breaks).
|
||||
//
|
||||
// SetMetricGrid / SetUserPageBreak target a region's staff-based slot, so they
|
||||
// share `layout_region_slot`: the region must be live *and* staff-based (a
|
||||
// FreeGraphic region has neither a metric grid nor a break list). That verdict
|
||||
// reads only the base-free indices, so reduce() and reduce_onto() agree on it.
|
||||
|
||||
fn set_metadata(&mut self, env: &OperationEnvelope, op: &SetMetadataOp) -> OperationEffect {
|
||||
let effect = match &self.last_metadata {
|
||||
Some((prev_op, prev_meta)) if self.concurrent(env.id, *prev_op) => {
|
||||
if *prev_meta == op.metadata {
|
||||
return OperationEffect::NoOp {
|
||||
reason: NoOpReason::AlreadyApplied,
|
||||
};
|
||||
}
|
||||
let prev_op = *prev_op;
|
||||
let conflict = ConflictRecord::new(
|
||||
ConflictKind::StructuralFieldCollision {
|
||||
winner: env.id,
|
||||
loser: prev_op,
|
||||
field: FieldPath("metadata".to_string()),
|
||||
},
|
||||
vec![env.id, prev_op],
|
||||
vec![],
|
||||
);
|
||||
let cid = conflict.id;
|
||||
self.conflicts.insert(conflict);
|
||||
OperationEffect::Conflicted { conflict: cid }
|
||||
}
|
||||
_ => OperationEffect::Applied,
|
||||
};
|
||||
self.last_metadata = Some((env.id, op.metadata.clone()));
|
||||
/// `Some(NoOp)` when `region` cannot carry a metric grid or user break — it is
|
||||
/// missing, tombstoned, or FreeGraphic; `None` when it has a staff-based slot.
|
||||
fn layout_region_slot(&self, region: RegionId) -> Option<OperationEffect> {
|
||||
let live = matches!(
|
||||
self.objects.get(&TypedObjectId::Region(region)),
|
||||
Some(ObjectState::Live)
|
||||
);
|
||||
if live && self.staff_based_regions.contains(®ion) {
|
||||
None
|
||||
} else {
|
||||
Some(OperationEffect::NoOp {
|
||||
reason: NoOpReason::PreconditionFailedUnderReduction {
|
||||
reason: PreconditionFailureReason::TargetMissing,
|
||||
},
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
fn set_metadata(&mut self, op: &SetMetadataOp) -> OperationEffect {
|
||||
// Advisory LWW: no conflict, no idempotence short-circuit. The resolved
|
||||
// value is the last write in canonical order, held in the graph.
|
||||
if let Some(score) = self.graph.as_mut() {
|
||||
score.metadata = op.metadata.clone();
|
||||
}
|
||||
effect
|
||||
OperationEffect::Applied
|
||||
}
|
||||
|
||||
fn set_metric_grid(
|
||||
|
|
@ -1354,15 +1352,28 @@ impl<'a> Reducer<'a> {
|
|||
env: &OperationEnvelope,
|
||||
op: &SetMetricGridOp,
|
||||
) -> OperationEffect {
|
||||
if !matches!(
|
||||
self.objects.get(&TypedObjectId::Region(op.region)),
|
||||
Some(ObjectState::Live)
|
||||
) {
|
||||
return OperationEffect::NoOp {
|
||||
reason: NoOpReason::PreconditionFailedUnderReduction {
|
||||
reason: PreconditionFailureReason::TargetMissing,
|
||||
},
|
||||
};
|
||||
if let Some(effect) = self.layout_region_slot(op.region) {
|
||||
return effect;
|
||||
}
|
||||
// A non-empty grid names a time signature per meter change; the graph
|
||||
// invariant (epiphany-core invariants.rs) rejects a grid that references an
|
||||
// undeclared signature, so reject it here rather than install an
|
||||
// invariant-violating grid. Time signatures are seeded from the base, so
|
||||
// this verdict is identical with or without a graph.
|
||||
if let Some(grid) = &op.grid {
|
||||
for change in &grid.meter_sequence {
|
||||
if !matches!(
|
||||
self.objects
|
||||
.get(&TypedObjectId::TimeSignature(change.time_signature)),
|
||||
Some(ObjectState::Live)
|
||||
) {
|
||||
return OperationEffect::NoOp {
|
||||
reason: NoOpReason::PreconditionFailedUnderReduction {
|
||||
reason: PreconditionFailureReason::TargetMissing,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
let prev = self
|
||||
.last_metric_grid
|
||||
|
|
@ -1401,53 +1412,23 @@ impl<'a> Reducer<'a> {
|
|||
return;
|
||||
};
|
||||
if let Some(region) = score.canvas.regions.iter_mut().find(|r| r.id == region) {
|
||||
match &mut region.content {
|
||||
epiphany_core::RegionContent::StaffBased(content) => {
|
||||
content.default_metric_grid = grid.clone();
|
||||
}
|
||||
epiphany_core::RegionContent::Hybrid { staves, .. } => {
|
||||
staves.default_metric_grid = grid.clone();
|
||||
}
|
||||
epiphany_core::RegionContent::FreeGraphic(_) => {}
|
||||
if let Some(content) = region.content.staff_based_mut() {
|
||||
content.default_metric_grid = grid.clone();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn set_user_page_break(&mut self, op: &SetUserPageBreakOp) -> OperationEffect {
|
||||
if let Some(effect) = self.layout_region_slot(op.region) {
|
||||
return effect;
|
||||
}
|
||||
if let Some(score) = self.graph.as_mut() {
|
||||
let Some(region) = score
|
||||
.canvas
|
||||
.regions
|
||||
.iter_mut()
|
||||
.find(|region| region.id == op.region)
|
||||
else {
|
||||
return OperationEffect::NoOp {
|
||||
reason: NoOpReason::PreconditionFailedUnderReduction {
|
||||
reason: PreconditionFailureReason::TargetMissing,
|
||||
},
|
||||
};
|
||||
};
|
||||
let breaks = match &mut region.content {
|
||||
epiphany_core::RegionContent::StaffBased(content) => &mut content.user_page_breaks,
|
||||
epiphany_core::RegionContent::Hybrid { staves, .. } => &mut staves.user_page_breaks,
|
||||
epiphany_core::RegionContent::FreeGraphic(_) => {
|
||||
return OperationEffect::NoOp {
|
||||
reason: NoOpReason::PreconditionFailedUnderReduction {
|
||||
reason: PreconditionFailureReason::TargetMissing,
|
||||
},
|
||||
}
|
||||
if let Some(region) = score.canvas.regions.iter_mut().find(|r| r.id == op.region) {
|
||||
if let Some(content) = region.content.staff_based_mut() {
|
||||
apply_break_lww(&mut content.user_page_breaks, &op.anchor, op.present);
|
||||
}
|
||||
};
|
||||
let anchor = op.anchor.clone();
|
||||
if op.present {
|
||||
if !breaks.contains(&anchor) {
|
||||
breaks.push(anchor);
|
||||
}
|
||||
} else {
|
||||
breaks.retain(|candidate| candidate != &anchor);
|
||||
}
|
||||
}
|
||||
|
||||
self.page_breaks
|
||||
.insert((op.region, op.resolved_position()), op.present);
|
||||
OperationEffect::Applied
|
||||
|
|
@ -2075,6 +2056,9 @@ impl<'a> Reducer<'a> {
|
|||
self.graph_create_region(&op.region);
|
||||
self.mint_container(env, robj);
|
||||
self.region_instances.entry(op.region_id()).or_default();
|
||||
if op.region.content.staff_based().is_some() {
|
||||
self.staff_based_regions.insert(op.region_id());
|
||||
}
|
||||
OperationEffect::Applied
|
||||
}
|
||||
|
||||
|
|
@ -2177,6 +2161,7 @@ impl<'a> Reducer<'a> {
|
|||
},
|
||||
);
|
||||
self.region_instances.remove(&op.region);
|
||||
self.staff_based_regions.remove(&op.region);
|
||||
self.graph_delete_region(op.region);
|
||||
OperationEffect::Applied
|
||||
}
|
||||
|
|
@ -3201,11 +3186,11 @@ impl<'a> Reducer<'a> {
|
|||
last_event_modify: self.last_event_modify.clone(),
|
||||
last_pitch_modify: self.last_pitch_modify.clone(),
|
||||
last_cross_cutting_modify: self.last_cross_cutting_modify.clone(),
|
||||
last_metadata: self.last_metadata.clone(),
|
||||
last_metric_grid: self.last_metric_grid.clone(),
|
||||
structures: self.structures.clone(),
|
||||
region_instances: self.region_instances.clone(),
|
||||
instance_voices: self.instance_voices.clone(),
|
||||
staff_based_regions: self.staff_based_regions.clone(),
|
||||
migrated_regions: self.migrated_regions.clone(),
|
||||
region_migrator: self.region_migrator.clone(),
|
||||
descriptors: self.descriptors.clone(),
|
||||
|
|
@ -3227,11 +3212,11 @@ impl<'a> Reducer<'a> {
|
|||
self.last_event_modify = s.last_event_modify;
|
||||
self.last_pitch_modify = s.last_pitch_modify;
|
||||
self.last_cross_cutting_modify = s.last_cross_cutting_modify;
|
||||
self.last_metadata = s.last_metadata;
|
||||
self.last_metric_grid = s.last_metric_grid;
|
||||
self.structures = s.structures;
|
||||
self.region_instances = s.region_instances;
|
||||
self.instance_voices = s.instance_voices;
|
||||
self.staff_based_regions = s.staff_based_regions;
|
||||
self.migrated_regions = s.migrated_regions;
|
||||
self.region_migrator = s.region_migrator;
|
||||
self.descriptors = s.descriptors;
|
||||
|
|
|
|||
|
|
@ -269,8 +269,9 @@ pub fn region(id: RegionId) -> Region {
|
|||
}
|
||||
|
||||
/// Score metadata with a `nth`-distinct title (M2d) — distinct `nth` give
|
||||
/// distinct [`ScoreMetadata`] values so a harness can make concurrent
|
||||
/// `SetMetadata`s agree or conflict deterministically.
|
||||
/// distinct [`ScoreMetadata`] values so a harness can drive concurrent
|
||||
/// `SetMetadata`s, an advisory LWW field that resolves by canonical order with
|
||||
/// no conflict.
|
||||
pub fn score_metadata(nth: u8) -> epiphany_core::ScoreMetadata {
|
||||
epiphany_core::ScoreMetadata {
|
||||
title: Some(format!("title-{nth}")),
|
||||
|
|
|
|||
|
|
@ -1297,7 +1297,7 @@ fn score_settings_materialize_in_the_graph_and_ledger() {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn concurrent_differing_set_metadata_conflicts() {
|
||||
fn concurrent_differing_set_metadata_is_advisory_lww() {
|
||||
let base = epiphany_core::generators::valid_score(100);
|
||||
// Two concurrent SetMetadata (neither sees the other) with differing values.
|
||||
let a = envelope(
|
||||
|
|
@ -1321,16 +1321,297 @@ fn concurrent_differing_set_metadata_conflicts() {
|
|||
})),
|
||||
);
|
||||
let mut set = OperationSet::new();
|
||||
set.accept_all(vec![a, b]);
|
||||
set.accept_all(vec![a.clone(), b.clone()]);
|
||||
let result = set.reduce_onto(&base);
|
||||
assert_eq!(
|
||||
result.state.conflicts.records().len(),
|
||||
1,
|
||||
"concurrent differing SetMetadata records exactly one conflict"
|
||||
|
||||
// Metadata is an advisory last-writer-wins field: a clean concurrent edit
|
||||
// raises no conflict and leaves the materialized state clean (matching the
|
||||
// catalog/core-spec "LWW advisory" classification).
|
||||
assert!(
|
||||
result.state.conflicts.records().is_empty(),
|
||||
"concurrent differing SetMetadata is advisory — it records no conflict"
|
||||
);
|
||||
assert!(
|
||||
result.state.is_clean(),
|
||||
"an advisory metadata edit keeps the materialized state clean"
|
||||
);
|
||||
assert!(
|
||||
result
|
||||
.state
|
||||
.effects
|
||||
.iter()
|
||||
.all(|(_, effect)| matches!(effect, OperationEffect::Applied)),
|
||||
"both writes apply; the last in canonical order silently wins"
|
||||
);
|
||||
|
||||
// The resolved value is one of the two writes and is permutation-independent.
|
||||
let resolved = result.score.metadata.clone();
|
||||
assert!(
|
||||
resolved == valuegen::score_metadata(1) || resolved == valuegen::score_metadata(2),
|
||||
"the resolved metadata is one of the concurrent writes"
|
||||
);
|
||||
let mut reversed = OperationSet::new();
|
||||
reversed.accept_all(vec![b, a]);
|
||||
assert_eq!(
|
||||
reversed.reduce_onto(&base).score.metadata,
|
||||
resolved,
|
||||
"metadata resolution is independent of acceptance order"
|
||||
);
|
||||
assert!(check_invariants(&result.score).is_empty());
|
||||
}
|
||||
|
||||
/// Whether an effect is the precondition NoOp the layout ops use for a target
|
||||
/// that is missing, tombstoned, or not staff-based.
|
||||
fn is_target_missing(effect: &OperationEffect) -> bool {
|
||||
matches!(
|
||||
effect,
|
||||
OperationEffect::NoOp {
|
||||
reason: NoOpReason::PreconditionFailedUnderReduction {
|
||||
reason: PreconditionFailureReason::TargetMissing,
|
||||
},
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn set_user_page_break_on_a_missing_region_is_a_consistent_noop() {
|
||||
let base = epiphany_core::generators::valid_score(100);
|
||||
let ghost = epiphany_core::RegionId::new(ReplicaId(123), 7); // absent from the base
|
||||
let op = envelope(
|
||||
58,
|
||||
0,
|
||||
10,
|
||||
CausalContext::new(),
|
||||
None,
|
||||
OperationPayload::Primitive(OperationKind::SetUserPageBreak(
|
||||
epiphany_ops::SetUserPageBreakOp {
|
||||
region: ghost,
|
||||
anchor: valuegen::region_start_anchor(
|
||||
ghost,
|
||||
MusicalPosition(RationalTime::from_int(0)),
|
||||
),
|
||||
present: true,
|
||||
},
|
||||
)),
|
||||
);
|
||||
let mut set = OperationSet::new();
|
||||
set.accept(op.clone());
|
||||
|
||||
// Graph-aware: the absent region has no break slot, so the op is a NoOp and
|
||||
// nothing enters the canonical page_breaks.
|
||||
let graph = set.reduce_onto(&base);
|
||||
assert!(is_target_missing(&effect_of(&graph, op.id)));
|
||||
assert!(
|
||||
graph.state.page_breaks.is_empty(),
|
||||
"no canonical page break is recorded for a missing region"
|
||||
);
|
||||
|
||||
// Base-free: with no region ever minted the reducer reaches the same verdict,
|
||||
// so reduce() and reduce_onto() agree.
|
||||
let bookkeeping = set.reduce();
|
||||
let effect = bookkeeping
|
||||
.effects
|
||||
.iter()
|
||||
.find(|(e, _)| *e == op.id)
|
||||
.map(|(_, eff)| eff)
|
||||
.expect("the operation has an effect");
|
||||
assert!(is_target_missing(effect));
|
||||
assert!(bookkeeping.page_breaks.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn layout_ops_on_a_free_graphic_region_are_rejected() {
|
||||
let mut base = epiphany_core::generators::valid_score(100);
|
||||
// A staff-less FreeGraphic region: it has neither a metric-grid nor a break
|
||||
// slot, so both layout ops must reject it. The staff-based index is read with
|
||||
// or without a graph, so reduce() and reduce_onto() reach the same verdict.
|
||||
let fg_id = epiphany_core::RegionId::new(ReplicaId(99), 0);
|
||||
let mut fg = valuegen::region(fg_id);
|
||||
fg.content = epiphany_core::RegionContent::FreeGraphic(epiphany_core::GraphicContent {
|
||||
objects: Vec::new(),
|
||||
});
|
||||
base.canvas.regions.push(fg);
|
||||
|
||||
let page = envelope(
|
||||
59,
|
||||
0,
|
||||
10,
|
||||
CausalContext::new(),
|
||||
None,
|
||||
OperationPayload::Primitive(OperationKind::SetUserPageBreak(
|
||||
epiphany_ops::SetUserPageBreakOp {
|
||||
region: fg_id,
|
||||
anchor: valuegen::region_start_anchor(
|
||||
fg_id,
|
||||
MusicalPosition(RationalTime::from_int(0)),
|
||||
),
|
||||
present: true,
|
||||
},
|
||||
)),
|
||||
);
|
||||
let grid = envelope(
|
||||
59,
|
||||
1,
|
||||
11,
|
||||
CausalContext::new().with_seen(ReplicaId(59), 0),
|
||||
None,
|
||||
OperationPayload::Primitive(OperationKind::SetMetricGrid(
|
||||
epiphany_ops::SetMetricGridOp {
|
||||
region: fg_id,
|
||||
grid: Some(valuegen::metric_grid()),
|
||||
},
|
||||
)),
|
||||
);
|
||||
let mut set = OperationSet::new();
|
||||
set.accept_all(vec![page.clone(), grid.clone()]);
|
||||
let result = set.reduce_onto(&base);
|
||||
|
||||
assert!(
|
||||
is_target_missing(&effect_of(&result, page.id)),
|
||||
"a page break on a FreeGraphic region is rejected"
|
||||
);
|
||||
assert!(
|
||||
is_target_missing(&effect_of(&result, grid.id)),
|
||||
"a metric grid on a FreeGraphic region is rejected"
|
||||
);
|
||||
assert!(
|
||||
result.state.page_breaks.is_empty(),
|
||||
"nothing is recorded for the FreeGraphic region"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn set_metric_grid_rejects_an_undeclared_time_signature_reference() {
|
||||
let base = epiphany_core::generators::valid_score(100);
|
||||
let region = base.canvas.regions[0].id;
|
||||
let grid_before = base.canvas.regions[0]
|
||||
.content
|
||||
.staff_based()
|
||||
.expect("fixture is staff based")
|
||||
.default_metric_grid
|
||||
.clone();
|
||||
|
||||
// A grid whose single meter change names a time signature the score never
|
||||
// declares — the graph invariant (epiphany-core) forbids installing it.
|
||||
let bogus = epiphany_core::TimeSignatureId::new(ReplicaId(200), 1);
|
||||
let bad_grid = epiphany_core::MetricGrid {
|
||||
meter_sequence: vec![epiphany_core::MeterChange {
|
||||
anchor: valuegen::region_start_anchor(
|
||||
region,
|
||||
MusicalPosition(RationalTime::from_int(0)),
|
||||
),
|
||||
time_signature: bogus,
|
||||
}],
|
||||
};
|
||||
let op = envelope(
|
||||
60,
|
||||
0,
|
||||
10,
|
||||
CausalContext::new(),
|
||||
None,
|
||||
OperationPayload::Primitive(OperationKind::SetMetricGrid(
|
||||
epiphany_ops::SetMetricGridOp {
|
||||
region,
|
||||
grid: Some(bad_grid),
|
||||
},
|
||||
)),
|
||||
);
|
||||
let mut set = OperationSet::new();
|
||||
set.accept(op.clone());
|
||||
let result = set.reduce_onto(&base);
|
||||
|
||||
assert!(
|
||||
is_target_missing(&effect_of(&result, op.id)),
|
||||
"a grid referencing an undeclared time signature is rejected"
|
||||
);
|
||||
let grid_after = result
|
||||
.score
|
||||
.canvas
|
||||
.regions
|
||||
.iter()
|
||||
.find(|r| r.id == region)
|
||||
.expect("the region survives")
|
||||
.content
|
||||
.staff_based()
|
||||
.expect("still staff based")
|
||||
.default_metric_grid
|
||||
.clone();
|
||||
assert_eq!(
|
||||
grid_before, grid_after,
|
||||
"the rejected grid leaves the region's metric grid unchanged"
|
||||
);
|
||||
assert!(check_invariants(&result.score).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_breaks_at_one_resolved_position_collapse_to_a_single_anchor() {
|
||||
let base = epiphany_core::generators::valid_score(100);
|
||||
let region = base.canvas.regions[0].id;
|
||||
let offset = RationalTime::from_int(4);
|
||||
// Two structurally distinct anchors (region start vs. end) that resolve to the
|
||||
// *same* musical position — the canonical LWW key — both set present.
|
||||
let start_anchor = TimeAnchor::Region {
|
||||
id: region,
|
||||
edge: RegionEdge::Start,
|
||||
offset: AnchorOffset::Musical(MusicalDuration(offset.clone())),
|
||||
};
|
||||
let end_anchor = TimeAnchor::Region {
|
||||
id: region,
|
||||
edge: RegionEdge::End,
|
||||
offset: AnchorOffset::Musical(MusicalDuration(offset.clone())),
|
||||
};
|
||||
let first = envelope(
|
||||
61,
|
||||
0,
|
||||
10,
|
||||
CausalContext::new(),
|
||||
None,
|
||||
OperationPayload::Primitive(OperationKind::SetUserPageBreak(
|
||||
epiphany_ops::SetUserPageBreakOp {
|
||||
region,
|
||||
anchor: start_anchor,
|
||||
present: true,
|
||||
},
|
||||
)),
|
||||
);
|
||||
let second = envelope(
|
||||
61,
|
||||
1,
|
||||
11,
|
||||
CausalContext::new().with_seen(ReplicaId(61), 0),
|
||||
None,
|
||||
OperationPayload::Primitive(OperationKind::SetUserPageBreak(
|
||||
epiphany_ops::SetUserPageBreakOp {
|
||||
region,
|
||||
anchor: end_anchor.clone(),
|
||||
present: true,
|
||||
},
|
||||
)),
|
||||
);
|
||||
let mut set = OperationSet::new();
|
||||
set.accept_all(vec![first, second]);
|
||||
let result = set.reduce_onto(&base);
|
||||
|
||||
let breaks = &result
|
||||
.score
|
||||
.canvas
|
||||
.regions
|
||||
.iter()
|
||||
.find(|r| r.id == region)
|
||||
.expect("the region survives")
|
||||
.content
|
||||
.staff_based()
|
||||
.expect("fixture is staff based")
|
||||
.user_page_breaks;
|
||||
assert_eq!(
|
||||
breaks.as_slice(),
|
||||
&[end_anchor],
|
||||
"two anchors at one resolved position collapse to the single last writer"
|
||||
);
|
||||
assert_eq!(
|
||||
result.state.page_breaks.len(),
|
||||
1,
|
||||
"exactly one canonical LWW slot for the resolved position"
|
||||
);
|
||||
assert!(matches!(
|
||||
result.state.conflicts.records()[0].kind,
|
||||
ConflictKind::StructuralFieldCollision { .. }
|
||||
));
|
||||
assert!(check_invariants(&result.score).is_empty());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1633,6 +1633,10 @@ mod tests {
|
|||
(region_id(&mut rng), musical_position(&mut rng)),
|
||||
rng.boolean(),
|
||||
);
|
||||
state.page_breaks.insert(
|
||||
(region_id(&mut rng), musical_position(&mut rng)),
|
||||
rng.boolean(),
|
||||
);
|
||||
state
|
||||
.pending
|
||||
.push((operation_id(&mut rng), pending_reason(&mut rng)));
|
||||
|
|
|
|||
|
|
@ -715,7 +715,7 @@ pub fn gen_vertical_band(rng: &mut Rng) -> VerticalBand {
|
|||
/// An operation-kind tag (every variant Agent C's type provides, including the
|
||||
/// registered form).
|
||||
pub fn gen_operation_kind_tag(rng: &mut Rng) -> OperationKindTag {
|
||||
match rng.below(17) {
|
||||
match rng.below(24) {
|
||||
0 => OperationKindTag::InsertEvent,
|
||||
1 => OperationKindTag::DeleteEvent,
|
||||
2 => OperationKindTag::ModifyEvent,
|
||||
|
|
@ -732,6 +732,13 @@ pub fn gen_operation_kind_tag(rng: &mut Rng) -> OperationKindTag {
|
|||
13 => OperationKindTag::SetUserSystemBreak,
|
||||
14 => OperationKindTag::SetUserPageBreak,
|
||||
15 => OperationKindTag::DeclareTransaction,
|
||||
16 => OperationKindTag::InsertIdentifiedPitch,
|
||||
17 => OperationKindTag::DeleteIdentifiedPitch,
|
||||
18 => OperationKindTag::ModifyIdentifiedPitch,
|
||||
19 => OperationKindTag::CreateVoice,
|
||||
20 => OperationKindTag::DeleteVoice,
|
||||
21 => OperationKindTag::SetMetadata,
|
||||
22 => OperationKindTag::SetMetricGrid,
|
||||
_ => OperationKindTag::Registered(epiphany_ops::OperationKindRegistryId(
|
||||
rng.next_u64() as u128
|
||||
)),
|
||||
|
|
|
|||
Loading…
Reference in New Issue