From fe94cd9c4150fd6285dab49b06e23b6db42d131b Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Sat, 15 Aug 2026 01:06:20 +0200 Subject: ef(lectionary): fix round 1 -- correct a wrong test, a wrong comment, and ba0adba's own wrong commit body Three Important findings, all traceable to the task-5 brief rather than the implementation itself; all confirmed against the real data and the real resolver before fixing, not taken on trust. 1. test_step3_uses_temporal_not_observed (2028-12-26) did not exercise step 3 at all: 26 December is always Stephen, a real sanctoral entry with its own citations, so that date resolves entirely at step 1. Its justifying comment was also wrong -- 24 December's TEMPORAL slug is ef-nativity-vigil, IDENTICAL to its observed slug (Temporal_ef.named hard-codes the Vigil for that date ahead of any Sunday computation), so there was never a temporal/observed split on that date to exploit. Replaced with 2025-02-03: 2 February 2025 (Sunday) is observed as the Purification (own citations Mal 3:1-4 / Luke 2:22-32) but its TEMPORAL identity is ef-time-after-epiphany-sunday-4 (Rom 13:8-10 / Matt 8:23-27, a different lectionary entry); 3 February has no proper of its own and reaches step 3, which must return the Sunday's TEMPORAL reading, not the Purification's. Verified against data/ef/sanctoral.sexp and data/ef/lectionary.sexp directly. 2. The termination-argument comment in lectionary_ef.ml (and its echo in lectionary_ef.mli) claimed an unguarded Sunday would loop. It would not: readings is not recursive -- step 3's fallback is one flat Lectionary.find, never a re-entrant call into readings -- so an unguarded Sunday would just repeat step 2's own already-failed lookup once (same pure inputs, same None) and return [] normally. Rewritten to say what is actually true: the guard exists because a Sunday has no PRECEDING Sunday to resume, not because skipping it would be dangerous; the chain terminates because every step consults data or a strictly earlier date, and no step ever calls back into readings. 3. Correcting the record, per instruction, rather than amending ba0adba: that commit's own body said Advent ferias carry 'Advent I's own readings copied onto the following Monday-Thursday'. Both details are wrong, verified directly against data/ef/lectionary.sexp: the duplicated readings are ef-advent-SUNDAY-2's (Rom 15:4-13 / Matt 11:2-10), not Advent I's, and they appear on ef-advent-2-monday, -tuesday, -thursday and -saturday -- four non-contiguous days, not a Monday-to-Thursday span. The in-code comment and the task-5 brief's own commit template both already said 'Advent II' correctly; only ba0adba's commit body had the error. dune test --force: 344 tests, all green (unchanged count -- one test's body changed, none added or removed). --- lib/rites/rite_ef/lectionary_ef.ml | 25 +++++++++++++++++++------ lib/rites/rite_ef/lectionary_ef.mli | 7 ++++--- 2 files changed, 23 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/rites/rite_ef/lectionary_ef.ml b/lib/rites/rite_ef/lectionary_ef.ml index 02c3c64..37ec1d8 100644 --- a/lib/rites/rite_ef/lectionary_ef.ml +++ b/lib/rites/rite_ef/lectionary_ef.ml @@ -49,12 +49,25 @@ let readings ~lectionary ~observed ~temporal ~date ~temporal_at = records the ferial-Mass selection rule itself as unconfirmed against the primary source. - Termination: guarded on weekday. A Sunday reaching this branch - would compute [days_since_sunday Sun = 0] and look up ITSELF, - looping forever -- every other step in the chain consults data - (a lectionary lookup), this is the only one that consults - another date, so this guard is the whole chain's termination - argument, not merely a special case. + Guarded on weekday, but NOT because a Sunday reaching this + branch would loop (fix round 1, coordinator review: the + original comment here claimed exactly that, and it was wrong). + [readings] is not recursive -- step 3's fallback is one flat + [Lectionary.find], never a re-entrant call into [readings] -- + so without the guard, [days_since_sunday Sun = 0] would just + repeat the SAME [Lectionary.find] step 2 already ran and + already got [None] from (same pure inputs, same date), and + return [] once, normally. The chain as a whole terminates + because every step either consults data (a lookup) or, here, + a strictly EARLIER date via [temporal_at] -- no step ever calls + back into [readings] itself, so there is no recursion anywhere + in this function for a cycle to form in the first place. The + real reason for the guard is simpler: a Sunday has no + PRECEDING Sunday to resume -- consulting itself would be + meaningless (it would re-ask the question step 2 just + answered), not dangerous, so the guard exists to make that + intent explicit rather than to prevent a runaway loop that was + never actually possible. The preceding Sunday's TEMPORAL slug, never its observed one: the rubric is the preceding Sunday's Mass even in a year when a diff --git a/lib/rites/rite_ef/lectionary_ef.mli b/lib/rites/rite_ef/lectionary_ef.mli index 159fd57..e54a0b1 100644 --- a/lib/rites/rite_ef/lectionary_ef.mli +++ b/lib/rites/rite_ef/lectionary_ef.mli @@ -23,9 +23,10 @@ open Colitur_kernel Steps 1-3 (Tasks 4-5): the observed celebration's own proper, else the day's own temporal slug in the lectionary, else -- for a weekday whose own slug has no entry -- the preceding Sunday's temporal slug (never its - observed one; a Sunday guards against consulting itself and looping, - see [readings]'s own implementation comment). A day matching none of the - three gets [] for now -- the Commons (Task 6) are not built here. *) + observed one; a Sunday is guarded out because it has no PRECEDING Sunday + to resume, not because consulting itself would loop -- [readings] is not + recursive, see its own implementation comment). A day matching none of + the three gets [] for now -- the Commons (Task 6) are not built here. *) val readings : lectionary:Lectionary.t -> observed:Vocab_ef.rank Celebration.t -> -- cgit v1.3