From af297881e2eadeb5b7a88734b0fea5ab52de93cd Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Fri, 26 Jun 2026 21:58:31 -0400 Subject: [PATCH] Agent I-3: criterion-6 round-trip and golden lock on the real Engraver Criterion 6 (the Chapter 7 layout round-trip) and the render goldens previously exercised only the verbatim StubSolver, so a regression in the real Engraver's geometry could land unseen. I-3 drives both through the Engraver. - round_trip_with factors the solver-agnostic provenance contract out of round_trip (now a one-line stub wrapper): coverage, the complete Provenance surviving constrained -> resolved -> render, the source surjection, and no duplicate stable ids hold for *any* conformant solver. The Stub tier's verbatim-geometry clause is gated behind solver.tier() == Stub; every other tier re-spaces. The status gate accepts any renderable status (Solved / SolvedWithWarnings / PartialBudgetExhausted), not exactly Solved, so the helper matches its "arbitrary conformant solver" contract while still rejecting the diagnostic-only statuses that carry no authoritative layout. - criterion_six_round_trips_through_the_engravers_respacing (epiphany-engrave) runs the full graph -> logical -> constrained -> *engraved* -> render round trip over the criterion-6 hand-off fixtures -- ten_measure_single_staff (the measured fixture) and valid_score_rich (cross-cutting tuplet/tie/spanner), plus valid_score for breadth -- and asserts the whole provenance contract survives the Engraver's re-spacing. A non-vacuity check confirms the Engraver genuinely moved geometry, so provenance is preserved *through* a real geometry change -- the statement the verbatim stub can never make. This adds an epiphany-testkit dev-dep (no cycle: testkit does not depend on this crate). - The render-svg engraver acceptance test is upgraded from invariant-only to byte-locked: new .engrave.snapshot.txt / .engrave.svg goldens for both fixtures capture the Engraver's re-spaced output (e.g. ten_measure view_box width 82.26 vs the stub's 88.88, same glyph/stroke/class counts), so an Engraver geometry regression is caught at the byte level. A companion test asserts the engrave goldens genuinely differ from the stub goldens, catching the degeneracy where the Engraver echoes the stub (which would otherwise pass both golden checks independently). Also corrects the epiphany-engrave package description, which still claimed it reports SolverTier::Stub until it earns Minimal (it earned Minimal in I-2). Full gate green: build, fmt, clippy, 580 tests, conformance scale 1. Co-Authored-By: Claude Opus 4.8 --- Cargo.lock | 1 + crates/epiphany-engrave/Cargo.toml | 6 +- crates/epiphany-engrave/src/lib.rs | 58 +++++++++ crates/epiphany-layout-ir/src/lib.rs | 2 +- crates/epiphany-layout-ir/src/roundtrip.rs | 82 ++++++++----- .../epiphany-render-svg/tests/acceptance.rs | 73 ++++++++++-- ..._measure_single_staff.engrave.snapshot.txt | 14 +++ .../ten_measure_single_staff.engrave.svg | 110 ++++++++++++++++++ .../valid_score_rich.engrave.snapshot.txt | 14 +++ .../tests/golden/valid_score_rich.engrave.svg | 52 +++++++++ 10 files changed, 371 insertions(+), 41 deletions(-) create mode 100644 crates/epiphany-render-svg/tests/golden/ten_measure_single_staff.engrave.snapshot.txt create mode 100644 crates/epiphany-render-svg/tests/golden/ten_measure_single_staff.engrave.svg create mode 100644 crates/epiphany-render-svg/tests/golden/valid_score_rich.engrave.snapshot.txt create mode 100644 crates/epiphany-render-svg/tests/golden/valid_score_rich.engrave.svg diff --git a/Cargo.lock b/Cargo.lock index b736934..e6055e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -99,6 +99,7 @@ version = "0.0.0" dependencies = [ "epiphany-core", "epiphany-layout-ir", + "epiphany-testkit", ] [[package]] diff --git a/crates/epiphany-engrave/Cargo.toml b/crates/epiphany-engrave/Cargo.toml index 19320ed..dda1df1 100644 --- a/crates/epiphany-engrave/Cargo.toml +++ b/crates/epiphany-engrave/Cargo.toml @@ -5,7 +5,7 @@ edition.workspace = true rust-version.workspace = true authors.workspace = true repository.workspace = true -description = "Agent I's Epiphany engraving solver (spec Chapter 9): turns a ConstrainedLayoutIR into a ResolvedLayoutIR with real geometry. Phase-2 scaffold — a deterministic horizontal-spacing pass (the first axis of the planned two-pass spring layout) that honestly reports SolverTier::Stub until it evaluates the declared hard constraints and earns the Minimal tier." +description = "Agent I's Epiphany engraving solver (spec Chapter 9): turns a ConstrainedLayoutIR into a ResolvedLayoutIR with real geometry. A deterministic horizontal-spacing pass (the first axis of the planned two-pass spring layout) that evaluates the declared hard constraints and reports SolverTier::Minimal. The vertical spring pass and casting-off are deferred to a later tier." [dependencies] # The solver consumes/produces the Chapter 7 IR stages and implements the @@ -17,3 +17,7 @@ epiphany-layout-ir.workspace = true # Tests drive the solver from real score fixtures via epiphany-core's generators # (and a couple of leaf id/time types). epiphany-core.workspace = true +# The criterion-6 round-trip test mirrors the hand-off gate's fixtures, including +# epiphany-testkit's `ten_measure_single_staff` (testkit does not depend on this +# crate, so this dev-dep introduces no cycle). +epiphany-testkit.workspace = true diff --git a/crates/epiphany-engrave/src/lib.rs b/crates/epiphany-engrave/src/lib.rs index 2de5e90..51e3a60 100644 --- a/crates/epiphany-engrave/src/lib.rs +++ b/crates/epiphany-engrave/src/lib.rs @@ -919,4 +919,62 @@ mod tests { ); assert_eq!(full.layout, inc.layout); } + + /// Acceptance criterion 6 (the Chapter 7 layout round-trip) against the **real** + /// Engraver, not the verbatim stub. `round_trip_with` drives graph -> logical -> + /// constrained -> *engraved* -> render and asserts the whole provenance contract + /// internally: every laid-out object is covered, the complete `Provenance` + /// (source, synthesis, dependencies, stable id) survives the engrave pass and the + /// render unchanged, the recovered source set is exactly the set laid out, and no + /// two objects share a stable id. The point the stub can never make: provenance is + /// preserved *through a real geometry change* — the Engraver re-spaces every glyph + /// and the strokes that track them, yet not one back-reference is lost. + #[test] + fn criterion_six_round_trips_through_the_engravers_respacing() { + use epiphany_core::generators::valid_score; + use epiphany_layout_ir::{round_trip_with, SolveStatus}; + use epiphany_testkit::fixtures::ten_measure_single_staff; + + for seed in 0..32u64 { + // Mirror the criterion-6 hand-off gate's own fixtures — the 10-measure + // single staff (measures + barlines) and the rich score (cross-cutting + // tuplet/tie/spanner/marker) — and keep `valid_score` for added breadth. + let scores = [ + ten_measure_single_staff(seed), + valid_score(seed), + valid_score_rich(seed), + ]; + for score in scores { + // round_trip_with asserts the full provenance contract; a Solved + // status also confirms the Engraver satisfied the pipeline's hard + // constraints (the stub pipeline declares none, so vacuously). + let report = round_trip_with(&score, &Engraver); + assert_eq!(report.status, SolveStatus::Solved); + } + } + + // Non-vacuity: the contract above held *through* a genuine re-spacing — the + // Engraver's geometry differs from the stub's verbatim columns, so provenance + // survived a real geometry change rather than a pass-through. + let constrained = to_constrained(&to_logical(&valid_score_rich(11))); + assert!(constrained.glyphs.len() >= 2); + let engraved = Engraver + .solve(&constrained, &SolverConfig::default()) + .layout; + let stub = StubSolver + .solve(&constrained, &SolverConfig::default()) + .layout; + assert_ne!( + engraved + .glyphs + .iter() + .map(|g| g.position.x.0) + .collect::>(), + stub.glyphs + .iter() + .map(|g| g.position.x.0) + .collect::>(), + "the Engraver must re-space, not echo the stub's verbatim columns" + ); + } } diff --git a/crates/epiphany-layout-ir/src/lib.rs b/crates/epiphany-layout-ir/src/lib.rs index cd2c51b..9d23ef7 100644 --- a/crates/epiphany-layout-ir/src/lib.rs +++ b/crates/epiphany-layout-ir/src/lib.rs @@ -130,7 +130,7 @@ pub use render::{ pub use resolved::{ ResolvedGlyph, ResolvedLayoutIR, ResolvedMeasure, ResolvedPage, ResolvedStaff, ResolvedSystem, }; -pub use roundtrip::{laid_out_object_ids, round_trip, RoundTripReport}; +pub use roundtrip::{laid_out_object_ids, round_trip, round_trip_with, RoundTripReport}; pub use solver::{ ConstraintId, ConstraintSolver, ExtensionMetric, ExtensionMetricId, ExtensionWarningId, InvalidationScope, InvalidationSet, NormalizedMetric, QualityMetricKind, QualityMetricVector, diff --git a/crates/epiphany-layout-ir/src/roundtrip.rs b/crates/epiphany-layout-ir/src/roundtrip.rs index 3f3026a..0587729 100644 --- a/crates/epiphany-layout-ir/src/roundtrip.rs +++ b/crates/epiphany-layout-ir/src/roundtrip.rs @@ -22,7 +22,7 @@ use crate::constrained::to_constrained; use crate::logical::{cross_cutting_objects, identified_pitch_ids, to_logical, LayoutObject}; use crate::provenance::{LayoutObjectId, Provenance}; use crate::render::to_render; -use crate::solver::{ConstraintSolver, SolveStatus, SolverConfig, StubSolver}; +use crate::solver::{ConstraintSolver, SolveStatus, SolverConfig, SolverTier, StubSolver}; /// The set of score-graph objects the pipeline lays out — the [`TypedObjectId`]s /// the round-trip expects to recover from the RenderIR. Kept in lockstep with @@ -111,7 +111,22 @@ fn provenance_map<'a>( /// laid out — a surjection onto graph identity (every laid-out source is /// recovered and nothing spurious appears; one source may back several layout /// objects, which the distinct stable ids account for). +/// +/// Runs against the [`StubSolver`]; [`round_trip_with`] runs the same contract +/// against any conformant solver (a real solver re-spaces, so the verbatim-geometry +/// clause is checked only for the [`SolverTier::Stub`] tier). pub fn round_trip(score: &Score) -> RoundTripReport { + round_trip_with(score, &StubSolver) +} + +/// Criterion 6 for an arbitrary conformant `solver`: every provenance-preservation +/// guarantee of [`round_trip`] *except* the verbatim-geometry clause, which is the +/// [`SolverTier::Stub`] tier's specific promise. A real solver re-spaces the glyphs, +/// but the [`Provenance`] back-references — `source`, `synthesis`, `dependencies`, +/// and `stable_id` — must survive that re-spacing unchanged, and the recovered +/// source set must still be exactly the set laid out. This is the strictly stronger +/// statement: a solver may move geometry, never lose a provenance trace. +pub fn round_trip_with(score: &Score, solver: &S) -> RoundTripReport { let logical = to_logical(score); let constrained = to_constrained(&logical); @@ -148,42 +163,51 @@ pub fn round_trip(score: &Score) -> RoundTripReport { ); } - let report = StubSolver.solve(&constrained, &SolverConfig::default()); - assert_eq!( - report.status, - SolveStatus::Solved, - "the stub solver must report Solved" + let report = solver.solve(&constrained, &SolverConfig::default()); + // A conformant solver need not report exactly `Solved`: any renderable status + // (`Solved`, `SolvedWithWarnings`, `PartialBudgetExhausted`) carries a layout + // whose hard constraints are satisfied (Chapter 9 §"The Solver Report"), which + // is all the round-trip needs — the provenance contract below is independent of + // quality. The non-renderable, diagnostic-only statuses (Unsatisfiable, + // InternalError) have no authoritative layout to round-trip. + assert!( + report.status.is_renderable(), + "the solver must return a renderable layout, got {:?}", + report.status ); assert!( report.satisfied_hard_constraints, - "the stub solver must satisfy all hard constraints" + "the solver must satisfy all hard constraints" ); - // The stub solver's geometry contract: it returns the input geometry - // *verbatim* — each resolved glyph's position is exactly its constrained - // baseline (Chapter 9 / QUICKSTART: "the input geometry verbatim"). The - // solver preserves order, so glyphs line up by index. - assert_eq!( - report.layout.glyphs.len(), - constrained.glyphs.len(), - "the solver must not add or drop glyphs" - ); - for (constrained_glyph, resolved_glyph) in constrained.glyphs.iter().zip(&report.layout.glyphs) - { + // The Stub tier's geometry contract: it returns the input geometry *verbatim* + // — each resolved glyph's position is exactly its constrained baseline (Chapter + // 9 / QUICKSTART: "the input geometry verbatim"), strokes pass through in order. + // A higher tier re-spaces, so this clause is the stub's alone; provenance + // preservation (asserted below) holds for *every* tier regardless. + if solver.tier() == SolverTier::Stub { assert_eq!( - resolved_glyph.position, constrained_glyph.baseline, - "stub solver must return the input geometry verbatim" + report.layout.glyphs.len(), + constrained.glyphs.len(), + "the stub solver must not add or drop glyphs" + ); + for (constrained_glyph, resolved_glyph) in + constrained.glyphs.iter().zip(&report.layout.glyphs) + { + assert_eq!( + resolved_glyph.position, constrained_glyph.baseline, + "stub solver must return the input geometry verbatim" + ); + assert_eq!(resolved_glyph.glyph, constrained_glyph.glyph); + assert_eq!(resolved_glyph.bounding_box, constrained_glyph.bounding_box); + assert_eq!(resolved_glyph.style, constrained_glyph.style); + assert_eq!(resolved_glyph.layer, constrained_glyph.layer); + } + assert_eq!( + report.layout.strokes, constrained.strokes, + "stub solver must return the input strokes verbatim" ); - assert_eq!(resolved_glyph.glyph, constrained_glyph.glyph); - assert_eq!(resolved_glyph.bounding_box, constrained_glyph.bounding_box); - assert_eq!(resolved_glyph.style, constrained_glyph.style); - assert_eq!(resolved_glyph.layer, constrained_glyph.layer); } - // Strokes likewise pass through the stub verbatim, in order. - assert_eq!( - report.layout.strokes, constrained.strokes, - "stub solver must return the input strokes verbatim" - ); let resolved_map = provenance_map( "resolved", diff --git a/crates/epiphany-render-svg/tests/acceptance.rs b/crates/epiphany-render-svg/tests/acceptance.rs index 272f1b0..f5ad078 100644 --- a/crates/epiphany-render-svg/tests/acceptance.rs +++ b/crates/epiphany-render-svg/tests/acceptance.rs @@ -51,9 +51,14 @@ fn pipeline(score: &Score) -> (ConstrainedLayoutIR, RenderOutput) { /// A deterministic, human-diffable serialization of the machine acceptance /// snapshot. -fn snapshot_text(fixture: &str, constrained: &ConstrainedLayoutIR, out: &RenderOutput) -> String { +fn snapshot_text( + fixture: &str, + solver: &str, + constrained: &ConstrainedLayoutIR, + out: &RenderOutput, +) -> String { let mut s = String::new(); - s.push_str(&format!("fixture={fixture} solver=stub\n")); + s.push_str(&format!("fixture={fixture} solver={solver}\n")); s.push_str(&format!("glyph_count={}\n", out.stats.glyph_count)); s.push_str(&format!("path_count={}\n", out.stats.path_count)); s.push_str(&format!( @@ -162,7 +167,7 @@ fn fixtures_render_to_golden_locked_svg_and_snapshot() { // Golden locks: the machine snapshot and the full SVG bytes. assert_golden( &format!("{fixture}.stub.snapshot.txt"), - &snapshot_text(fixture, &constrained, &out), + &snapshot_text(fixture, "stub", &constrained, &out), ); assert_golden(&format!("{fixture}.stub.svg"), &out.svg); } @@ -252,15 +257,29 @@ fn svg_validates_under_xmllint_when_available() { let _ = std::fs::remove_file(&tmp); } +/// Drives the full pipeline through Agent I's real `Engraver` — whose +/// horizontal-spacing pass re-spaces every glyph and the strokes that track them. +fn engrave_pipeline(score: &Score) -> (ConstrainedLayoutIR, RenderOutput) { + use epiphany_engrave::Engraver; + + let constrained = to_constrained(&to_logical(score)); + let layout = Engraver + .solve(&constrained, &SolverConfig::default()) + .layout; + let out = render(&layout, &RenderOptions::default()); + (constrained, out) +} + /// The renderer's contract is "consumes *any* solver's `ResolvedLayoutIR`", but -/// the goldens above only exercise the stub. This drives Agent I's real +/// the `.stub.*` goldens above only exercise the stub. This drives the real /// `Engraver` — whose horizontal-spacing pass produces geometry that *differs* -/// from the stub's verbatim columns — through the same renderer and asserts the -/// output is still well-formed with every glyph drawn and traced, so an -/// `Engraver` geometry regression cannot slip through unnoticed (the stub -/// goldens would not catch it). +/// from the stub's verbatim columns — through the same renderer and **golden-locks +/// its SVG bytes and machine snapshot** under `.engrave.*`, so an `Engraver` +/// geometry regression is caught at the byte level (the stub goldens cannot see +/// it), while the same structural invariants (every glyph drawn, every primitive +/// traced, well-formed XML, no fallbacks) hold as for the stub. #[test] -fn engraver_output_renders_well_formed_with_every_glyph_drawn() { +fn engraver_output_is_golden_locked_well_formed_with_every_glyph_drawn() { use epiphany_engrave::Engraver; use epiphany_layout_ir::SolveStatus; @@ -272,8 +291,11 @@ fn engraver_output_renders_well_formed_with_every_glyph_drawn() { SolveStatus::Solved, "{fixture}: engrave should solve the constraint-free stub pipeline" ); - let out = render(&report.layout, &RenderOptions::default()); + let (_, out) = engrave_pipeline(&score); + + // Structural invariants — identical to the stub's, because the Engraver + // re-spaces geometry without adding, dropping, or un-tracing a primitive. check_well_formed(&out.svg) .unwrap_or_else(|e| panic!("{fixture}: engraver SVG is not well-formed: {e}")); assert!(out.stats.glyph_count > 0, "{fixture}: nothing was laid out"); @@ -291,5 +313,36 @@ fn engraver_output_renders_well_formed_with_every_glyph_drawn() { out.diagnostics.is_empty(), "{fixture}: stub-pipeline glyphs are all bundled, so no fallback is expected" ); + + // Provenance survival into the SVG, as for the stub goldens. + for g in &out_glyph_ids(&score) { + assert!( + out.svg.contains(&format!("data-prov=\"{g:032x}\"")), + "{fixture}: provenance {g:032x} did not survive the engrave into the SVG" + ); + } + + // Golden locks: the engraver's machine snapshot and full SVG bytes. + assert_golden( + &format!("{fixture}.engrave.snapshot.txt"), + &snapshot_text(fixture, "engrave", &constrained, &out), + ); + assert_golden(&format!("{fixture}.engrave.svg"), &out.svg); + } +} + +/// The two solvers' goldens must genuinely differ: the Engraver re-spaces, so its +/// SVG cannot be byte-identical to the stub's verbatim-column SVG. A regression +/// that made the Engraver echo the stub would pass both golden checks above (each +/// would just match its own committed file) — this catches that degeneracy. +#[test] +fn engraver_goldens_differ_from_the_stub_goldens() { + for (fixture, score) in fixtures() { + let (_, stub_out) = pipeline(&score); + let (_, engrave_out) = engrave_pipeline(&score); + assert_ne!( + stub_out.svg, engrave_out.svg, + "{fixture}: the Engraver must re-space, so its SVG must differ from the stub's" + ); } } diff --git a/crates/epiphany-render-svg/tests/golden/ten_measure_single_staff.engrave.snapshot.txt b/crates/epiphany-render-svg/tests/golden/ten_measure_single_staff.engrave.snapshot.txt new file mode 100644 index 0000000..d3ae548 --- /dev/null +++ b/crates/epiphany-render-svg/tests/golden/ten_measure_single_staff.engrave.snapshot.txt @@ -0,0 +1,14 @@ +fixture=ten_measure_single_staff solver=engrave +glyph_count=51 +path_count=51 +fallback_rect_count=0 +stroke_count=51 +provenance_count=102 +layer_count=1 +hard_constraint_count=0 +xml_well_formed=true +view_box=[-3.0113542 -3.632 82.26242 11.632] +class_counts: + barline=10 + clef=1 + notehead=40 diff --git a/crates/epiphany-render-svg/tests/golden/ten_measure_single_staff.engrave.svg b/crates/epiphany-render-svg/tests/golden/ten_measure_single_staff.engrave.svg new file mode 100644 index 0000000..d6fc091 --- /dev/null +++ b/crates/epiphany-render-svg/tests/golden/ten_measure_single_staff.engrave.svg @@ -0,0 +1,110 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/crates/epiphany-render-svg/tests/golden/valid_score_rich.engrave.snapshot.txt b/crates/epiphany-render-svg/tests/golden/valid_score_rich.engrave.snapshot.txt new file mode 100644 index 0000000..2d34da6 --- /dev/null +++ b/crates/epiphany-render-svg/tests/golden/valid_score_rich.engrave.snapshot.txt @@ -0,0 +1,14 @@ +fixture=valid_score_rich solver=engrave +glyph_count=11 +path_count=11 +fallback_rect_count=0 +stroke_count=33 +provenance_count=44 +layer_count=1 +hard_constraint_count=0 +xml_well_formed=true +view_box=[-3.0113542 -3.632 27.468542 11.632] +class_counts: + barline=1 + clef=3 + notehead=7 diff --git a/crates/epiphany-render-svg/tests/golden/valid_score_rich.engrave.svg b/crates/epiphany-render-svg/tests/golden/valid_score_rich.engrave.svg new file mode 100644 index 0000000..3d5f346 --- /dev/null +++ b/crates/epiphany-render-svg/tests/golden/valid_score_rich.engrave.svg @@ -0,0 +1,52 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +