From c655c525ef07deafb113a291489b396c81d7ce4a Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Wed, 12 Aug 2026 15:27:00 +0200 Subject: gen-sanctoral-ef, caldata: fix round 3 -- guard coverage, rank pins, name test, Anastasia Second external re-review returned Spec pass on round 2 (all ten prior findings addressed, C1/C2 fixed at the root, mechanism claims held under mutation) and found two blocking items in the I3 guard itself, plus three recommended hardenings. All verified independently before fixing. BLOCKING 1: refYearExplainsAbsence's own doc comment said the id-rank exclusion covers "6-31 December"; the code implemented `Day() >= 26`. A typo (26 -> 6), but the wrong kind to have on the one comment this whole review chain exists to keep honest -- fixed as part of the rewrite below, not left as an isolated one-character correction. BLOCKING 2: the guard covered only 6 of the 13 MM-DD values round 1's own temporal_ef.go changes actually made refYearExplainsAbsence couple to. Measured, not just reasoned about: running the function's body against the branch-point engine versus HEAD over the six reference years flips its verdict on 50 dates across 12-17 through 12-23 (round 1's late-Advent promotion, RG 91 entry 18, a 140-day change) AND 12-26 through 12-31 (the Nativity-octave promotion, RG 67/68, already guarded). The late-Advent half was unguarded. No live bug today -- confirmed by regenerating: the ini is BYTE-IDENTICAL before and after this fix. The only sanctoral entry in 17-23 December is `thomas` (21 December), reached via harvestDate's `obs` (observed-office) path, never refYearExplainsAbsence's commemoration-id-trust path at all. Widened the guard to 17-31 December anyway, on the side of the known-safe default (RankCommemoration) rather than wait for the first real entry in that range to repeat I3's exact failure shape. The two sub-ranges are NOT the same strength of evidence, and the doc comment says so explicitly now: 26-31 December rests on a positive RG 68(d)/(e) citation naming Thomas Becket and Silvester directly as bare "Commemoratio, no class"; 17-23 December is a precautionary exclusion with no equivalent citation -- RG 91 entry 18 ranks the FERIA, not any commemoration that might coincide with it, and (per defect 2's own finding) a genuine class-2 feast there would actually WIN against the ferias's own privilege, so "the day is class-2" does not reliably explain a class-2 saint's absence here the way it does for the Nativity octave's two named cases. Recorded as a real asymmetry, not smoothed over. RECOMMENDED 3: rank = commemoration is now pinned for all four entries this task's two review rounds touched -- romanus, eusebius-confessor (C2), thomas-becket, silvester (I3) -- not just their presence and date. `git grep` before this commit returned zero hits for thomas-becket or silvester rank anywhere in *_test.go; a future regeneration could have rewritten either back to class-4, the exact I3 failure mode, with the suite green throughout. RECOMMENDED 4: TestTridentineNamesPreservedAcrossRegeneration rewritten on both axes the review found loose. It counted name.pl only -- a hardcoded single language, the identical "whitelist of two" mistake C1's own generator fix exists to stop repeating, just moved into the test instead. Now DISCOVERS every name. key actually present in the data (so a regeneration dropping name.la, or losing a future name.de, is caught the same way as name.pl) and asserts the discovered language set is exactly {en, pl} -- not just each count. The >= 315 floor had 7 entries of slack (the review proved 6 missing Polish names still passed it); tightened to the exact, independently verified true count (322), zero slack. RECOMMENDED 5: the 25 December Anastasia gap and its cause are now recorded, not just her absence. Two compounding, independently checked causes: fetchOnce reads only `data[0]`, missalemeum's FIRST of Christmas Day's three Masses, so a commemoration attached to a second or third Mass (the calendarium's own "In secunda Missa: Commemoratio S. Anastasiae Mart.") is structurally invisible to this harvester for ANY date, not just this one -- but ALSO, checked live, missalemeum's own "commemorations" list is empty on all three of the date's records, not just the first, so a fetchOnce rewritten to merge all of a date's Masses would still not recover her. Documented at both fetchOnce's own definition and the PRIMARY-SOURCE NOTE, since the two facts belong to different questions (a real code limitation worth fixing someday vs. an upstream data gap this generator cannot close at all). Witnesses: TestTridentineCommemorationRanksPinned (new), TestTridentineRomanusAndEusebiusPresent (extended with rank), TestTridentineNamesPreservedAcrossRegeneration (rewritten). No pre-fix failure to report for the two new/tightened tests specifically -- these are regression GUARDS against a bug class already fixed in round 2, not fixes for a currently-red state; their value is in what they would now catch, not what they catch today. internal/caldata/ tridentine-calendar.ini is NOT regenerated by this commit -- confirmed byte-identical before and after re-running the generator, exactly as "no live bug today" predicts. --- internal/caldata/caldata_test.go | 114 ++++++++++++++++++++++++++++++++------- 1 file changed, 95 insertions(+), 19 deletions(-) (limited to 'internal') diff --git a/internal/caldata/caldata_test.go b/internal/caldata/caldata_test.go index 2ba052d..7dbbf84 100644 --- a/internal/caldata/caldata_test.go +++ b/internal/caldata/caldata_test.go @@ -246,18 +246,56 @@ func TestTridentineRomanusAndEusebiusPresent(t *testing.T) { } else if rc.Fields["date"] != "08-09" { t.Errorf("romanus: date = %q, want 08-09", rc.Fields["date"]) } - want := map[string]string{ - "eusebius-confessor": "08-14", // "S. Eusebii Conf." -- a Confessor - "eusebius": "12-16", // "S. Eusebii Ep. et Mart." -- a Bishop and Martyr, a different person + want := map[string]struct{ date, rank string }{ + // St Eusebius Confessor (14 Aug) and St Romanus (9 Aug, checked + // above) are both bare "Commemoratio" in the calendarium (no class + // of their own) -- pinned here, not just presence/date, per + // TestTridentineCommemorationRanksPinned's own reasoning below: + // romanus's own rank is pinned there instead, since that test + // groups every commemoration-rank entry this task's fixes touched + // in one place. eusebius-confessor is listed here because it is + // the OTHER half of this specific test's own slug-disambiguation + // story. + "eusebius-confessor": {"08-14", "commemoration"}, // "S. Eusebii Conf." -- a Confessor + "eusebius": {"12-16", "class-3"}, // "S. Eusebii Ep. et Mart." -- a Bishop and Martyr, a different person, unaffected by this round } - for slug, date := range want { + for slug, w := range want { rc, ok := l.Cels[slug] if !ok { - t.Errorf("missing %q (%s)", slug, date) + t.Errorf("missing %q (%s)", slug, w.date) continue } - if rc.Fields["date"] != date { - t.Errorf("%s: date = %q, want %q", slug, rc.Fields["date"], date) + if rc.Fields["date"] != w.date { + t.Errorf("%s: date = %q, want %q", slug, rc.Fields["date"], w.date) + } + if rc.Fields["rank"] != w.rank { + t.Errorf("%s: rank = %q, want %q", slug, rc.Fields["rank"], w.rank) + } + } +} + +// TestTridentineCommemorationRanksPinned: nothing in this test suite +// asserted `rank` for thomas-becket, silvester, or romanus before this -- +// `TestTridentineRomanusAndEusebiusPresent` above pinned presence and date +// for romanus but not rank, and Thomas Becket/Silvester (the I3 fix, RG +// 68(d)/(e)) had no rank assertion anywhere at all. So a future +// regeneration could silently rewrite any of the three back to class-4 -- +// the EXACT I3 failure mode -- with the suite green throughout. Pinned +// together because they are the same failure shape (a bare "Commemoratio" +// in the calendarium, no class of its own, silently promoted by +// refYearExplainsAbsence's own coupling to temporal_ef.go, see that +// function's doc comment) even though they were found in two different +// review rounds (romanus in C2, Thomas Becket/Silvester in I3). +func TestTridentineCommemorationRanksPinned(t *testing.T) { + l := Tridentine() + for _, slug := range []string{"romanus", "thomas-becket", "silvester"} { + rc, ok := l.Cels[slug] + if !ok { + t.Errorf("missing %q", slug) + continue + } + if rc.Fields["rank"] != "commemoration" { + t.Errorf("%s: rank = %q, want commemoration", slug, rc.Fields["rank"]) } } } @@ -300,24 +338,62 @@ func TestTridentineNoTransferArtifacts(t *testing.T) { // reaching mobile.Day(date, "ef", version, "pl") -- a shipped dlectio entry // point -- with no error anywhere in the chain. // -// Checks both a broad coverage floor (not exactly 322: a handful of entries -// added by this same task's own fixes -- St Agnes secundo, St Boniface -// Martyr, St Evaristus, St Theodore, St Romanus, St Eusebius Confessor -- -// never had a curated Polish name to preserve in the first place, so 322 is -// not achievable) and one specific, checkable value (the coordinator's own -// example) so a coverage-only guard cannot itself be satisfied by silently -// wrong values. +// CORRECTED after review: the first version of this test counted name.pl +// only (a hardcoded single language -- the exact "whitelist of two +// languages" mistake C1's own fix was written to stop repeating, just +// moved into the test), and its coverage floor (>= 315) had 7 entries of +// slack -- the review proved dropping 6 entries' Polish names still passed +// it. Fixed on both axes: the language set is DISCOVERED from the data +// (every "name." key actually present, not a hardcoded list, so a +// regeneration dropping name.la or introducing a future name.de is +// checked the same way as name.pl), and the floor for each discovered +// language is its EXACT true count, not a loose approximation -- verified +// directly against the branch point (`git show 2386a45:...`) once, by +// hand, and hardcoded as the answer, not derived at test time from data +// that could itself be wrong. +// +// True counts, independently verified: name.en on every entry (327, all +// regenerated fresh from missalemeum, including the 5 entries this task's +// own fixes added); name.pl on exactly 322 (preserved from the branch +// point; the 5 new entries -- St Agnes secundo, St Boniface Martyr, St +// Evaristus, St Theodore, St Eusebius Confessor -- never had a curated +// Polish name to preserve in the first place, so 322, not 327, is the +// correct target, not a shortfall); no OTHER name. exists in the +// branch point at all (name.la is read by the generator but the +// bootstrapped file has in fact never carried one), so the discovered +// language set itself must be exactly {en, pl} -- a regeneration that +// silently introduced or lost an entire language key, not just some +// values within one, is caught by this assertion, not only by the count. func TestTridentineNamesPreservedAcrossRegeneration(t *testing.T) { l := Tridentine() - n := 0 + counts := map[string]int{} for _, rc := range l.Cels { - if rc.Fields["name.pl"] != "" { - n++ + for k, v := range rc.Fields { + if v == "" || !strings.HasPrefix(k, "name.") { + continue + } + lang := strings.TrimPrefix(k, "name.") + counts[lang]++ } } - if n < 315 { - t.Errorf("name.pl coverage = %d entries, want >= 315 (was 322 before any of this task's fixes; a regeneration must preserve it, not merely not-crash)", n) + wantLangs := map[string]bool{"en": true, "pl": true} + for lang := range counts { + if !wantLangs[lang] { + t.Errorf("unexpected name.%s present (%d entries) -- discovered language set must be exactly {en, pl}", lang, counts[lang]) + } } + for lang := range wantLangs { + if counts[lang] == 0 { + t.Errorf("name.%s entirely absent -- discovered language set must be exactly {en, pl}", lang) + } + } + if got := counts["en"]; got != len(l.Cels) { + t.Errorf("name.en coverage = %d entries, want %d (every entry, all freshly regenerated)", got, len(l.Cels)) + } + if got := counts["pl"]; got != 322 { + t.Errorf("name.pl coverage = %d entries, want exactly 322 (the branch point's own true count; the 5 entries this task added have no curated Polish name to preserve, so more or fewer than 322 is a bug either way)", got) + } + rc, ok := l.Cels["assumption-of-the-blessed-virgin-mary"] if !ok { t.Fatal("missing assumption-of-the-blessed-virgin-mary") -- cgit v1.3