From 39c8684646f7bad7161b9eeae166b121cc580a3c Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Tue, 11 Aug 2026 15:14:10 +0200 Subject: kernel(validate): synthetic negative-path fixture; drop vacuous slug check Task 14 review, findings 1 and 2. Finding 1: the only tests against Validate exercised the clean path against real EF data, so the evidence that each check can actually fire lived in a scratch mutation probe that was never committed. A future edit that quietly weakened a check would leave the suite green, since a weaker check only makes more inputs pass. Added a small synthetic two-season, two-rank rite fixture in test_validate.ml -- not EF -- letting each test violate exactly one invariant directly: a temporal that raises (coverage), a season that recurs (seasons), a week that decreases mid-run (week), a weekday that disagrees with Date.weekday (weekday), a rank absent from the declared vocab (rank), and a colour outside Colour.all (colour, via a same-representation Obj.magic value, safe here because the check compares by structural equality rather than pattern match). A clean-baseline test confirms the fixture itself reports zero failures before any mutation is applied. Each new test was verified non-vacuous by temporarily weakening its corresponding check in validate.ml, confirming the matching test fails, then reverting -- the same trap one level up, checked explicitly rather than assumed. Finding 2: removed the slug well-formedness check. Slug.t is a private string validated on every construction path, and to_string is the identity, so round-tripping an existing Slug.t can never fail -- the check was structurally incapable of firing. Folded the explanation into the comment block that already covers why slug uniqueness isn't checked, since it's the same kind of fact: a property the type system delivers, not one Validate needs to assert. The colour check stays: unlike slug, Colour.all is a hand-maintained list that can drift from the type, so it is only practically (not structurally) tautological, the same class as the rank closure check. --- lib/kernel/validate.ml | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'lib/kernel') diff --git a/lib/kernel/validate.ml b/lib/kernel/validate.ml index 9be030a..b11c05c 100644 --- a/lib/kernel/validate.ml +++ b/lib/kernel/validate.ml @@ -35,14 +35,18 @@ let run vocab ~year_start ~temporal ~year = (* Weekday agreement. *) if t.Temporal.weekday <> Date.weekday date then fail date "weekday" "temporal weekday disagrees with Date.weekday"; - (* Slug well-formed. Uniqueness *per date* needs no check: [temporal] - returns exactly one office by construction, which is the type - system delivering the invariant. Uniqueness *across the year* is - deliberately NOT asserted -- a resumed Sunday reuses an earlier - Epiphany key on purpose, so the check would be false. *) - (match Slug.of_string (Slug.to_string cel.Celebration.slug) with - | Ok _ -> () - | Error e -> fail date "slug" e); + (* Slug: three properties, none checked here, all delivered + elsewhere. Well-formedness needs no check: [Slug.t] is a private + string validated on every construction path ([of_string], + [of_string_exn], [t_of_sexp]), and [to_string] is the identity, + so round-tripping an existing [Slug.t] can never fail -- a check + here would be structurally incapable of firing, which is worse + than no check, since it would look like coverage that isn't + there. Uniqueness *per date* needs no check either: [temporal] + returns exactly one office by construction. Uniqueness *across + the year* is deliberately NOT asserted -- a resumed Sunday + reuses an earlier Epiphany key on purpose, so the check would be + false. *) (* Vocabulary closure. *) if not (List.exists (fun r -> vocab.Vocab.rank_to_string r = vocab.Vocab.rank_to_string cel.Celebration.rank) -- cgit v1.3