From 6b41415f17f3264454f6dcc13898948fd1902d0b Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Sat, 15 Aug 2026 01:59:11 +0200 Subject: ef(lectionary): fix round 1 -- the step-4 guard, and a miscited authority Review re-verified all fifteen saints and all seven formularies against both printings and found zero citation discrepancies, confirming the Gabriel printing error and all three proper/Common reversals; the reordering was adjudicated sound. Two Important findings, both about the guard rather than the data. IMPORTANT 1 -- the guard had no test, and a comment claimed it did. `test_step4_never_diverts_a_temporal_office` asserted 2026-07-04 keeps its ferial Mass and its comment claimed to "pin it behaviourally". It did not: no shipped assignment names a temporal slug, so the guard is a NO-OP on shipped data and that test passes with the guard removed. The reviewer proved it by forcing `sanctoral_office = true` -- all 357 tests stayed green. That mattered more than an ordinary missing test: the guard is the structural claim the whole reordering argument leans on, and on a task with no oracle the tests are the entire safety net. Fixed by exposing `Commons.of_tables` (already present internally; it applies exactly `load`'s validation, so a synthetic table is a well-formed table with a WRONG assignment rather than malformed data the loader would reject before the guard is reached -- and it is the constructor a future diocesan overlay needs anyway), adding `day_with ~commons` beside `day`, and writing test_step4_guard_refuses_a_common_assigned_to_a_ferial_slug: a synthetic Commons assigning the Common of Abbots to `ef-time-after-pentecost-5-saturday`, a really-observed ferial slug, asserting 2026-07-04 still says its ferial Mass. Two checks, the first making the second mean something -- it asserts the hostile assignment IS present and resolvable, so the ferial Mass wins because the GUARD refused it and not because the lookup missed. Mutation-verified: with the guard disabled exactly one test fails, this one; before this round that mutation reddened nothing. The old test is renamed test_step4_leaves_a_temporal_office_alone_on_shipped_data and its comment corrected to say it does not pin the guard. IMPORTANT 2 -- the guard cited an authority that does not say what was claimed. The comment said "[Validate] already asserts slug uniqueness per liturgical year, so a sanctoral feast can never collide with a temporal slug". validate.ml's check maps over `Temporal.office` slugs ONLY, compared to each other across the days of one year; it says nothing about cross-stream collision. An assumption was promoted to an asserted invariant -- the exact defect class this project's citation discipline exists to catch. Rewritten to state it as an assumption (no sanctoral slug carries the `ef-` prefix every temporal slug does; 0 of 327 today), to quote the real authority verbatim (validate.ml's own note that this "is the same assumption the rest of this codebase already leans on"), to carry a CORRECTED marker naming the wrong citation so it is not re-derived, and to record the failure mode if it ever broke: benign and one-directional -- a colliding saint is denied his Common and falls through to steps 2/3, i.e. to exactly the answer the unguarded chain would have given him. No day gains a reading it should not have. MINOR -- the previous commit body led with "Blast radius, measured ... ZERO differing lines", which is true but oversold: the CLI prints no citations, so that sweep is STRUCTURALLY INCAPABLE of observing the reorder or any citation value. It is a valid negative control for the sanctoral `Edit` overlay and nothing else. The evidence for the reorder is mutation 1 (relegating step 4 to last reddens exactly the four step-4 pins) plus those pins' own explicit expected values -- not the sweep. Correcting the record here. MINOR -- test_commons_load_rejects_bad_data wrote four FIXED-name files into the temp dir and removed none, colliding across concurrent runs. Replaced with a local `with_temp_file` copied from test_lectionary.ml's own (`Filename.temp_file` + `Fun.protect ~finally`); the four labels moved into the assertion messages where they belonged. Verified zero leftovers after a full run. MINOR -- added the sharper argument for the reorder, which makes the deviation obviously right rather than merely well-evidenced, and it now LEADS the branch comment as warrant (1), with the Missal citation demoted to a corroborating warrant (2): step 4 is STEP 1'S CONTINUATION. 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 -- St Joseph beating a competing temporal entry because his readings sit on his Celebration.t, and 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 did not merely disagree with the data, it contradicted step 1. 358 tests green (357 before), 359 with the exhaustive sweep. --- lib/rites/rite_ef/lectionary_ef.ml | 66 ++++++++++++++++++++++++++++++++----- lib/rites/rite_ef/lectionary_ef.mli | 12 +++++++ 2 files changed, 70 insertions(+), 8 deletions(-) (limited to 'lib') 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 -- cgit v1.3