diff options
| -rw-r--r-- | lib/kernel/calendar.ml | 104 | ||||
| -rw-r--r-- | test/test_calendar.ml | 90 |
2 files changed, 168 insertions, 26 deletions
diff --git a/lib/kernel/calendar.ml b/lib/kernel/calendar.ml index 27e564f..f2a03a7 100644 --- a/lib/kernel/calendar.ml +++ b/lib/kernel/calendar.ml @@ -305,33 +305,86 @@ let build_day (rite : ('s, 'r) Rite.t) (idx : 'r Layer.by_date) double-booking it in both would fail Task 12's "appears exactly once" reading of this day alone. *) (* Whether a transferred/injected candidate is genuinely accounted for at - its assigned target -- true if it won the day outright there (every - [Transfer]-disposed candidate this kernel produced before a rite could - transfer a [Celebration.status = Commemoration_only] one: a losing - FEAST, which RG 96's own "next day not I or II class" guarantees an - unblocked day to win once it arrives -- [occupant_of] alone answered - this), OR if it survives at the target as one of the day's own admitted + its assigned target -- true under any of THREE conditions, not two + (fix round 1, ef-major-litanies task -- the first version of this + function, and this comment, claimed two, missing exactly the same + shape one channel further out than the bug it had just fixed; see + below for how that was found). + + (1) It won the day outright there (every [Transfer]-disposed candidate + this kernel produced before a rite could transfer a + [Celebration.status = Commemoration_only] one: a losing FEAST, which + RG 96's own "next day not I or II class" guarantees an unblocked day + to win once it arrives -- [occupant_of] alone used to answer this, and + still would). + + (2) It survives at the target as one of the day's own admitted COMMEMORATIONS instead (a shape [place_transfers] itself never used to produce, because nothing before could dispose a [Commemoration_only] - candidate as [Transfer] -- {!Precedence.resolve} holds such a candidate - out of the band contest entirely, so it can never win a day outright, - only ever be commemorated on one; a rite is nonetheless free to - [Transfer] one to a named date, e.g. RG 80's Major Litanies, RG 81's - own "nihil fit in Officio" making [observed] structurally impossible - for it anywhere). [occupant_of] alone under-reports this second shape - as unsettled, which previously had no live witness to catch it: a - transferred candidate that only ever becomes a commemoration, never the - day's own office, was mislabelled here with [unconverged_reason] (a - hardcoded string, not a true read of the placement pass's own - convergence -- {!place_transfers} itself had already reached a fixed - point) and double-counted by {!Validate}'s own "duplicated" check - (sighted once in [omitted] here under that wrong label, and correctly - again in [commemorations] at its target) -- found by this kernel's own - exhaustive property sweep once a rite (Rite_ef, RG 80) first produced - this shape, not guessed at in advance. Kept rite-agnostic: nothing here - reads anything EF-specific, only {!Precedence.resolution}'s own - [observed]/[commemorations] fields, the same two channels - {!Liturgical_day.t} already promises never to lose. *) + candidate as [Transfer] -- {!Precedence.resolve} holds such a + candidate out of the band contest entirely, so it can never win a day + outright, only ever be commemorated on one; a rite is nonetheless free + to [Transfer] one to a named date, e.g. RG 80's Major Litanies, RG + 81's own "nihil fit in Officio" making [observed] structurally + impossible for it anywhere). The FIRST version of this function + stopped here, at (1) and (2) -- {!Liturgical_day.t}'s own doc comment + promises [observed] and [commemorations] are never silently lost, and + this read as "the same two channels", which is where the "two, not + three" miscount came from: that promise is about {!Liturgical_day.t}'s + OWN five fields, not an exhaustive account of every way + {!Precedence.resolve} can dispose of a candidate at one date. + + (3) It reaches the target and is disposed there as [Omit] by + {!Precedence.rules.disposition} itself (RG 33's vigil omission, RG + 16(a)'s Sunday suppression, RG 26's IV-class-feria omission, the + Lord-vs-Lord exclusion, ...) OR is offered to + {!Precedence.rules.admit} there but capped out by an admission-count + limit RG 111 itself imposes (e.g. a Class1 day admits only ONE + privileged commemoration; a second one due the same day, or the SAME + Litanies candidate no longer privileged, loses that slot) -- both + land in the target's own [omitted], with their own accurate, + already-diagnostic reason ("omitted: yielded to a higher day" / + "omitted: admission limit reached"), and both are just as genuinely + "delivered and considered" as (1)/(2), not stuck anywhere. Missing + this third channel reproduces the EXACT ORIGINAL BUG this function was + written to fix, one level further out: a candidate settled (via (3)) + at its target was still reported [unresolved] at its ORIGIN, under + the same wrong, hardcoded [unconverged_reason] label, and + double-counted by {!Validate}'s own "duplicated" check the same way. + + Found, not merely reasoned to: fix round 1's review reproduced it two + ways. Constructively, a second privileged [Commemoration_only] entry + placed on the Litanies' own transfer target (Easter+2) that sorts + ahead of it forces the Litanies to lose {!Precedence.rules.admit}'s + own Class1 "one privileged commemoration only" cap there. And, + already present in this branch's own mutation-testing record without + being run to ground at the time: mutation 3 (this task's own report, + `privilege_of`'s RG 109(f) branch forced to [Ordinary]) makes the + transferred Litanies itself lose that SAME Class1 cap at its OWN + target -- no second candidate needed, since an [Ordinary] commemoration + has no standing at all against a [Class1] day's privileged-only + admission rule ({!Precedence_ef.admit}'s own Class1 case). That + mutation's 9th failure, the exhaustive `Validate` property sweep on a + random year, was this bug; the report noted the failure and declined + to diagnose it before reverting the mutation, which is precisely how + it survived one fix round. + + Unreachable on shipped EF data today (every `Commemoration_only` + candidate this rite's own real data carries is at most [Class3] + except the Litanies themselves, and no second privileged + commemoration can ever fall on Easter+2 -- {!Precedence_ef + .privilege_of}'s own (a)-(e) categories are all Sunday/Ember/Advent- + Lent-Passiontide/Nativity-octave shaped, none of which Easter+2 is or + can be), which is exactly why it survived this far: nothing in the + shipped calendar has ever exercised it. Fails LOUDLY (a wrong, + misleading label) rather than silently, and is fixed here rather than + left as a documented residual, since the fix is a one-line + generalisation of the same check, not new machinery. Kept + rite-agnostic: nothing here reads anything EF-specific, only + {!Precedence.resolution}'s own [observed]/[commemorations]/[omitted] + fields -- THREE of {!Precedence.resolution}'s four fields (the fourth, + [deferred], denotes a candidate that has NOT yet settled at this date, + by definition, so it is correctly never consulted here). *) let settled_at target slug = let _, _, target_resolution = resolve_with_injected rite idx injected target in let matches (c : 'r Precedence.candidate) = @@ -339,6 +392,7 @@ let build_day (rite : ('s, 'r) Rite.t) (idx : 'r Layer.by_date) in matches target_resolution.Precedence.observed || List.exists (fun (c, _) -> matches c) target_resolution.Precedence.commemorations + || List.exists (fun (c, _) -> matches c) target_resolution.Precedence.omitted in let unresolved c = let slug = c.Precedence.cel.Celebration.slug in diff --git a/test/test_calendar.ml b/test/test_calendar.ml index 93cd0bb..22f5f04 100644 --- a/test/test_calendar.ml +++ b/test/test_calendar.ml @@ -426,6 +426,90 @@ let test_transfer_guard_records_failure_instead_of_looping () = in Alcotest.(check bool) "non-convergence is recorded rather than silently dropped or hung" true stuck +(* ef-major-litanies task, fix round 1 (F1) -- a regression test for a + THIRD settlement channel `build_day`'s own `settled_at` had to learn to + recognise: a transferred candidate that reaches its target and is then + CAPPED OUT there by the target day's own RG 111 admission-count limit + (not `observed`, not surviving as a `commemoration` -- the two channels + the first version of this fix checked), rather than settling cleanly. + Missing it reproduces the exact original bug ONE LEVEL FURTHER OUT: the + origin wrongly reports the transferred candidate as + `unconverged_reason`, even though placement genuinely converged. + + Unreachable on real EF data ALONE (the Major Litanies, RG 80, are the + only privileged `Commemoration_only` candidate real data carries, and + no second one can ever coincide with Easter+2) -- reproduced here the + same way the fix-round review did: one synthetic privileged + `Commemoration_only` candidate, added directly to the REAL EF layer + (not a hand-built synthetic rite -- this bug is about the real Litanies + candidate's own real transfer, so the real rite is the honest fixture), + on the real Litanies' own real 2011 transfer target (26 April -- Easter + 2011 = 24 April, so 25 April is Easter Monday, RG 80's second trigger, + landing on Easter+2 = 26 April), with a slug ("aaa-probe") sorting + ahead of "major-litanies" so it wins {!Rite_ef.Precedence_ef.admit}'s + Class1 "one privileged commemoration only" selection there, capping the + Litanies out. *) +let real_ef_layer_for_transfer_probes = + match Colitur_kernel.Layer.load Rite_ef.Vocab_ef.rank_of_sexp "../data/ef/sanctoral.sexp" with + | Error e -> failwith ("../data/ef/sanctoral.sexp: " ^ e) + | Ok layer -> ( + match Colitur_kernel.Overlay.load Rite_ef.Vocab_ef.rank_of_sexp "../data/ef/adjustments.sexp" with + | Error e -> failwith ("../data/ef/adjustments.sexp: " ^ e) + | Ok overlay -> + let layer, diagnostics = Colitur_kernel.Overlay.apply layer overlay in + if diagnostics <> [] then failwith "unexpected overlay diagnostics loading the real EF layer"; + layer) + +let test_transferred_commemoration_only_capped_out_at_target_settles_cleanly () = + let probe_date = + match Colitur_kernel.Date_spec.fixed ~month:4 ~day:26 with Ok d -> d | Error e -> failwith e + in + let probe = + { Colitur_kernel.Layer.date = probe_date; + cel = + Cel.make ~slug:(Sl.of_string_exn "aaa-probe") ~rank:Rite_ef.Vocab_ef.Class1 + ~status:Cel.Commemoration_only ~colour:Colitur_kernel.Colour.White + ~subject:Colitur_kernel.Subject.Saint ~layer:"synthetic-probe" () + } + in + let augmented_layer = Colitur_kernel.Layer.set real_ef_layer_for_transfer_probes probe in + (* Liturgical year "2010" (Advent 2010 -- eve of Advent 2011) covers both + 25 and 26 April 2011. *) + let year = C.year Rite_ef.context augmented_layer 2010 in + let find_date target = + match Array.to_list year |> List.find_opt (fun d -> D.compare d.LD.date target = 0) with + | Some d -> d + | None -> failwith "date not found in resolved year" + in + let origin = find_date (mk 2011 4 25) and target = find_date (mk 2011 4 26) in + let slug_s (c : Rite_ef.Vocab_ef.rank Cel.t) = Sl.to_string c.Cel.slug in + Alcotest.(check string) "2011-04-25 is Easter Monday, RG80's second trigger" "ef-easter-1-monday" + (slug_s origin.LD.observed); + let omitted_s d = List.map (fun (c, r) -> (slug_s c, r)) d.LD.omitted in + Alcotest.(check (list (pair string string))) "origin: major-litanies is NOT in [omitted] at all -- it \ + genuinely, cleanly transferred away, no false 'did not converge'" [] + (List.filter (fun (s, _) -> s = "major-litanies") (omitted_s origin)); + let contains_substring s ~needle = + let ls = String.length s and ln = String.length needle in + let rec at i = i + ln <= ls && (String.sub s i ln = needle || at (i + 1)) in + ln = 0 || at 0 + in + Alcotest.(check (list (pair string string))) "origin: no [omitted] entry anywhere claims non-convergence \ + (the exact regression this test guards against, stated directly rather than only via the slug check \ + above)" [] + (List.filter (fun (_, r) -> contains_substring r ~needle:"did not converge") (omitted_s origin)); + Alcotest.(check (list string)) "origin: [transferred_out] still correctly names major-litanies -> target" + [ "major-litanies->2011-04-26" ] + (List.map + (fun (c, d) -> Printf.sprintf "%s->%s" (slug_s c) (D.to_iso8601 d)) + origin.LD.transferred_out); + Alcotest.(check (list string)) "target: aaa-probe wins the Class1 privileged slot (sorts ahead of \ + major-litanies at the tied [unclassified] band)" [ "aaa-probe" ] + (List.map (fun (c, _) -> slug_s c) target.LD.commemorations); + Alcotest.(check bool) "target: major-litanies is capped out into [omitted] there, with the REAL \ + admission-limit reason, not lost and not mislabelled" true + (List.mem ("major-litanies", "omitted: admission limit reached") (List.map (fun (c,r) -> (slug_s c, r)) target.LD.omitted)) + let suite = ( "Calendar", [ Alcotest.test_case "year covers every day" `Quick test_year_covers_every_day; @@ -447,4 +531,8 @@ let suite = Alcotest.test_case "transfer target outside year is recorded not lost" `Quick test_transfer_target_outside_year_is_recorded_not_lost; Alcotest.test_case "transfer guard records failure instead of looping" `Quick - test_transfer_guard_records_failure_instead_of_looping ] ) + test_transfer_guard_records_failure_instead_of_looping; + Alcotest.test_case + "ef-major-litanies fix round 1 (F1): a transferred candidate capped out at its own target settles \ + cleanly, no false 'did not converge'" + `Quick test_transferred_commemoration_only_capped_out_at_target_settles_cleanly ] ) |