diff options
| -rw-r--r-- | lib/rites/rite_ef/lectionary_ef.ml | 66 | ||||
| -rw-r--r-- | lib/rites/rite_ef/lectionary_ef.mli | 12 | ||||
| -rw-r--r-- | test/test_lectionary_ef.ml | 106 |
3 files changed, 154 insertions, 30 deletions
diff --git a/lib/rites/rite_ef/lectionary_ef.ml b/lib/rites/rite_ef/lectionary_ef.ml index 2658cd3..329faff 100644 --- a/lib/rites/rite_ef/lectionary_ef.ml +++ b/lib/rites/rite_ef/lectionary_ef.ml @@ -125,8 +125,11 @@ let commons_for ~commons saint = Commons.find commons saint ferial-Mass rubrics when coding") -- that confirmation has not been done; do not read this comment as citing RG/the Missal for the SELECTION rule itself, only [Lectionary.find]'s presence-or-absence as the mechanism. - Step 4 is the one step of the four that DOES have a direct primary-source - warrant; its own comment gives it. *) + Step 4 is the one step of the four that does NOT rest on that shape: it + is step 1's own rule continued (the observed office's readings win over + the day's temporal ones, wherever colitur happens to store them), and it + is separately corroborated by a direct Missal citation. Its own comment + gives both. *) (* Days from a given weekday back to the preceding Sunday. Sunday itself yields 0, which is why step 3 must guard on it -- see [readings] below. *) let days_since_sunday : Date.weekday -> int = function @@ -160,8 +163,23 @@ let readings ~lectionary ~commons ~observed ~temporal ~date ~temporal_at = theirs -- not Septuagesima II Saturday's 2 Cor 11:19-33 / Luke 8:4-15, which is what the brief's ordering emits. - WARRANT, and it is the strongest in this chain: the Missal itself, - at each of these saints' own dates, names the Mass to be said -- + WARRANT (1) -- INTERNAL, and it is the decisive one: step 4 is + STEP 1'S CONTINUATION, not a fifth thing bolted after the temporal + fallbacks. Step 1 already runs the observed office's own proper + ahead of steps 2 and 3; step 4 is that same rule for the saints + whose readings the Missal keeps in a Common instead of printing on + the celebration. Placing it last would have made the chain + internally inconsistent with code that already existed -- one saint + (St Joseph, test_step1_wins_over_a_competing_step2_entry) beating a + competing temporal entry because his readings happen to sit on his + Celebration.t, and another (St Vincent Ferrer) losing to one + because his sit one indirection away. Nothing in the rubrics draws + that distinction; it is an artefact of where colitur stores the + data. So the plan's ordering was not merely wrong on the data + (below) -- it contradicted step 1. + + WARRANT (2) -- EXTERNAL, corroborating: the Missal itself, at each + of these saints' own dates, names the Mass to be said -- "Missa Cognovi, de Communi non Virginum II loco, praeter orationem sequentem" (9 March), "Missa Os iusti, de Communi Abbatum" (21 March), and so on. That is a direct instruction about what is @@ -173,16 +191,48 @@ let readings ~lectionary ~commons ~observed ~temporal ~date ~temporal_at = office read", a question that only governs when the temporal office is the one being celebrated. - The guard makes that precondition structural rather than a property + THE GUARD makes that precondition structural rather than a property of the data file: the Commons are consulted only when the observed celebration is not itself the day's temporal office. Without it, a future overlay that assigned a Common to a temporal slug by mistake would silently replace a feria's Mass; with it, ferias, Sundays, the Triduum and the RG 78 Saturday Office of the BVM (whose observed celebration IS its temporal office, deliberately sharing the ferial - slug) can never be diverted here at all. [Validate] already asserts - slug uniqueness per liturgical year, so a sanctoral feast can never - collide with a temporal slug and be wrongly excluded by it. *) + slug) can never be diverted here at all. + + The guard is a NO-OP on the shipped data -- no assignment in + data/ef/commons.sexp names a temporal slug -- which is exactly what + makes it defensive rather than load-bearing, and exactly why + nothing in the real data can exercise it. It is nevertheless + tested, against a SYNTHETIC [Commons.t] built by [Commons.of_tables] + that deliberately assigns a Common to a really-observed ferial slug: + test_step4_guard_refuses_a_common_assigned_to_a_ferial_slug. That + test fails with this guard removed; without it the guard had no + coverage at all (fix round 1, coordinator review -- the reviewer + forced [sanctoral_office = true] and the whole suite stayed green). + + COULD THE GUARD WRONGLY EXCLUDE A REAL SAINT? Only if a sanctoral + slug ever equalled the day's own temporal slug. It cannot today -- + every temporal slug [Temporal_ef] builds carries the "ef-" prefix + and 0 of the 327 entries in data/ef/sanctoral.sexp do -- but that + is an ASSUMPTION this codebase leans on, NOT an asserted invariant, + and it should not be dressed up as one. (CORRECTED, fix round 1: + this comment previously cited [Validate]'s slug-uniqueness check as + the authority. That check compares [Temporal.office] slugs to each + other across the days of one liturgical year -- it says nothing + about collision BETWEEN the temporal and sanctoral streams.) + [Validate] is candid about the same assumption where it makes it, + for a different purpose -- recovering a commemoration's origin, + validate.ml's own note: "This is exact whenever slugs cannot + collide across the two streams (Task 12's own 'observed' check + already assumes this for a different purpose), which is the same + assumption the rest of this codebase already leans on." + + The failure mode if that assumption ever broke is benign and + one-directional: a colliding saint would be denied his Common and + fall through to steps 2/3, i.e. to the temporal Mass -- which is + precisely the answer the unguarded chain would have given him + anyway. No day gains a reading it should not have. *) let sanctoral_office = not (Slug.equal observed.Celebration.slug temporal.Temporal.office.Celebration.slug) in diff --git a/lib/rites/rite_ef/lectionary_ef.mli b/lib/rites/rite_ef/lectionary_ef.mli index baf25d4..caefc74 100644 --- a/lib/rites/rite_ef/lectionary_ef.mli +++ b/lib/rites/rite_ef/lectionary_ef.mli @@ -53,6 +53,18 @@ module Commons : sig assignment naming a common that does not exist (likewise). *) val load : string -> (t, string) result + (** Builds a table directly, with exactly the validation {!load} applies to + a file (same four [Error]s, same canonical sort). Exposed so that a + caller can construct Commons from something other than a sexp file -- + a future diocesan overlay, or a test that needs a table the shipped + data deliberately does not contain. The guard on {!readings}' step 4 + is one such case: no shipped assignment names a temporal slug, so the + only way to exercise the guard at all is to build a table that does. *) + val of_tables : + commons:(Slug.t * Citation.t list) list -> + assigned:(Slug.t * Slug.t) list -> + (t, string) result + (** The Commons themselves, canonically sorted by id. *) val formularies : t -> (Slug.t * Citation.t list) list diff --git a/test/test_lectionary_ef.ml b/test/test_lectionary_ef.ml index e28476d..71888e3 100644 --- a/test/test_lectionary_ef.ml +++ b/test/test_lectionary_ef.ml @@ -64,6 +64,16 @@ let day y m d = (Rite_ef.context ~lectionary:(real_lectionary ()) ~commons:(real_commons ())) (real_layer ()) date +(* Same resolution, with a caller-supplied Commons table instead of the + committed one -- the seam the whole design rests on, and the only way to + exercise step 4's guard, which is deliberately a no-op on shipped data. *) +let day_with ~commons y m d = + let date = match Date.make ~year:y ~month:m ~day:d with + | Ok x -> x | Error e -> Alcotest.fail e in + Calendar.day + (Rite_ef.context ~lectionary:(real_lectionary ()) ~commons) + (real_layer ()) date + let refs (ld : _ Liturgical_day.t) = List.map (fun c -> c.Citation.reference) ld.citations @@ -272,19 +282,64 @@ let test_step1_proper_francis_of_paola () = [ "Phil 3:7-12"; "Luke 12:32-34" ] (refs (day 2008 4 2)) -(* Step 4 must never divert a day whose observed office IS its temporal - office. The guard in [readings] makes that structural; this pins it - behaviourally on the case most at risk -- the RG 78 Saturday Office of - the BVM, which deliberately REUSES the ordinary ferial slug (see - temporal_ef.ml's [bvm_saturday_names], "Slug" paragraph). If that shared - slug were ever assigned a Common, every feria sharing it would change - Mass. 4 July 2026 is such a Saturday; it keeps its own ferial Mass. *) -let test_step4_never_diverts_a_temporal_office () = +(* A day whose observed office IS its temporal office is untouched by step 4 + on the SHIPPED data. 4 July 2026 is such a day, and the case most at risk: + it is an RG 78 Saturday Office of the BVM, which deliberately REUSES the + ordinary ferial slug (temporal_ef.ml's [bvm_saturday_names], "Slug" + paragraph). + + This test alone does NOT pin the guard -- it passes with the guard + removed, because no shipped assignment names a temporal slug. It is the + baseline the next test needs, and it is labelled as such (fix round 1, + coordinator review: its comment previously claimed to "pin the guard + behaviourally", which was false -- the reviewer forced + [sanctoral_office = true] and all 357 tests stayed green). *) +let test_step4_leaves_a_temporal_office_alone_on_shipped_data () = Alcotest.(check (list string)) - "4 July 2026 (a BVM Saturday) keeps its ferial Mass, unaffected by step 4" + "4 July 2026 (a BVM Saturday) keeps its ferial Mass" [ "1 Pet 3:8-15."; "Matt 5:20-24." ] (refs (day 2026 7 4)) +(* THE GUARD, pinned. The guard is a no-op on shipped data by design, so the + only way to exercise it is to build the table it defends against: a + SYNTHETIC [Commons.t] that assigns a Common to a really-observed FERIAL + slug. That is exactly the future-overlay mistake the guard exists to + catch -- and note the blast radius it would have, since a ferial slug is + shared by every year in which that week and weekday coincide, not by one + date. + + Built through [Commons.of_tables], so it passes the same validation the + committed file does -- this is a well-formed table with a wrong + assignment, not malformed data caught by the loader. + + TEETH (verified, fix round 1): with the guard removed this returns the + Common of Abbots' Ecclus 45:1-6 / Matt 19:27-29 and the assertion fails. + The second check is what makes the first one mean something: it proves + the hostile assignment really is present and resolvable, so the ferial + Mass wins because the GUARD refused it, not because the lookup missed. *) +let test_step4_guard_refuses_a_common_assigned_to_a_ferial_slug () = + let ferial_slug = Slug.of_string_exn "ef-time-after-pentecost-5-saturday" in + let abbots = Slug.of_string_exn "common-of-abbots" in + let hostile = + match + Lectionary_ef.Commons.of_tables + ~commons: + [ (abbots, + [ { Citation.part = Citation.First; reference = "Ecclus 45:1-6" }; + { Citation.part = Citation.Gospel; reference = "Matt 19:27-29" } ]) ] + ~assigned:[ (ferial_slug, abbots) ] + with + | Ok c -> c + | Error e -> Alcotest.failf "synthetic commons failed to build: %s" e + in + Alcotest.(check bool) + "the synthetic table really does resolve that ferial slug to a Common" true + (Lectionary_ef.commons_for ~commons:hostile ferial_slug <> None); + Alcotest.(check (list string)) + "4 July 2026 keeps its ferial Mass even so: step 4 refuses a temporal office" + [ "1 Pet 3:8-15."; "Matt 5:20-24." ] + (refs (day_with ~commons:hostile 2026 7 4)) + (* THE DATA ASSERTION, and the one that cannot drift: after Task 6, every sanctoral entry that can ever BE the observed office -- i.e. every [status Feast] entry -- either carries its own proper or has a Common @@ -416,17 +471,22 @@ let test_step4_unreachable_propers_are_present () = Written against strings rather than the committed file so the committed file stays clean. *) let test_commons_load_rejects_bad_data () = - let write name contents = - let path = Filename.concat (Filename.get_temp_dir_name ()) name in + (* [Filename.temp_file] + [Fun.protect], exactly as test_lectionary.ml's + own [with_temp_file] does it (fix round 1, coordinator review: this + used four FIXED names in the temp dir and removed none of them, so + concurrent runs collided and the files leaked). *) + let with_temp_file contents f = + let path = Filename.temp_file "commons_test" ".sexp" in let oc = open_out path in output_string oc contents; close_out oc; - path + Fun.protect ~finally:(fun () -> Sys.remove path) (fun () -> f path) in - let err name contents = - match Lectionary_ef.Commons.load (write name contents) with - | Ok _ -> Alcotest.failf "%s: expected Error, got Ok" name - | Error e -> e + let err what contents = + with_temp_file contents (fun path -> + match Lectionary_ef.Commons.load path with + | Ok _ -> Alcotest.failf "%s: expected Error, got Ok" what + | Error e -> e) in let good_common = "(c-a (((part First) (reference \"A 1:1\")) ((part Gospel) (reference \"B 2:2\"))))" in let contains needle haystack = @@ -439,15 +499,15 @@ let test_commons_load_rejects_bad_data () = (contains needle msg) in check_msg "duplicate common" "duplicate common" - (err "commons-dup-common.sexp" + (err "duplicate common" (Printf.sprintf "((commons (%s %s)) (assigned ()))" good_common good_common)); check_msg "duplicate assignment" "duplicate assignment" - (err "commons-dup-assign.sexp" + (err "duplicate assignment" (Printf.sprintf "((commons (%s)) (assigned ((s c-a) (s c-a))))" good_common)); check_msg "empty formulary" "no citations" - (err "commons-empty.sexp" "((commons ((c-a ()))) (assigned ()))"); + (err "empty formulary" "((commons ((c-a ()))) (assigned ()))"); check_msg "unknown common" "unknown common" - (err "commons-unknown.sexp" + (err "unknown common" (Printf.sprintf "((commons (%s)) (assigned ((s c-missing))))" good_common)); (* A missing file is an [Error] too, never an exception -- the contract [Lectionary.load] already makes and the reason neither is read at @@ -487,8 +547,10 @@ let suite = test_step1_proper_beats_any_common_john_of_god); ("step 1: Thomas Aquinas's proper", `Quick, test_step1_proper_thomas_aquinas); ("step 1: Francis of Paola's proper", `Quick, test_step1_proper_francis_of_paola); - ("step 4 never diverts a temporal office (BVM Saturday)", `Quick, - test_step4_never_diverts_a_temporal_office); + ("step 4 leaves a temporal office alone on shipped data (BVM Saturday)", `Quick, + test_step4_leaves_a_temporal_office_alone_on_shipped_data); + ("step 4's GUARD refuses a Common wrongly assigned to a ferial slug", `Quick, + test_step4_guard_refuses_a_common_assigned_to_a_ferial_slug); ("every observable sanctoral Feast has a proper or a Common", `Quick, test_every_observable_sanctoral_feast_has_readings); ("the Commons assignment table, exactly", `Quick, test_step4_assignment_table); |