diff options
| author | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-08-11 15:14:10 +0200 |
|---|---|---|
| committer | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-08-11 15:14:10 +0200 |
| commit | 39c8684646f7bad7161b9eeae166b121cc580a3c (patch) | |
| tree | 5fc87f818e8d61dfa89185ea2b91438a37268e38 /lib/kernel/validate.ml | |
| parent | e15fe4952a569f08900f9d602f7bc98afd298542 (diff) | |
| download | colitur-39c8684646f7bad7161b9eeae166b121cc580a3c.tar.gz colitur-39c8684646f7bad7161b9eeae166b121cc580a3c.zip | |
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.
Diffstat (limited to 'lib/kernel/validate.ml')
| -rw-r--r-- | lib/kernel/validate.ml | 20 |
1 files changed, 12 insertions, 8 deletions
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) |
