Merge branch 'robustness-and-hackability'

An audit of the shipped program, plus the fixes it found.

The citation parser accepted OCaml integer-literal syntax, so a typo like
'Luke 1_1:5' silently became a different chapter. Four pairs of different
books shared a full title -- 1 and 2 Corinthians both rendered 'Epistola
ad Corinthios' -- leaving 108 citations in 2027 alone that a reader could
not resolve to a book. Spec section 8.5 is now delivered rather than
recorded: shipped styles re-parse their own output. Overlay errors no
longer name OCaml source files at the reader.

Also: the new-overlay scaffold shows citations at the right nesting
level, config --show validates before printing, error messages no longer
echo whole file lines, and a month answers to both spellings of its own
name.

4 files changed, 161 insertions, 5 deletions

diff --git a/test/cli.t b/test/cli.t
index b44a256..fe18800 100644
--- a/test/cli.t
+++ b/test/cli.t
@@ -1374,12 +1374,12 @@ fallback: `Colitur_citation.Render.with_book` takes a closed variant, not an
 arbitrary string, so anything other than "full"/"abbr" could never mean
 anything downstream.
 
+STDOUT STAYS EMPTY. The check used to happen partway down the table, so this
+exited 2 having already written five rows -- a caller redirecting stdout to a
+file got a truncated but plausible-looking report beside a non-zero status.
+Validation now runs before anything is printed.
+
   $ XDG_CONFIG_HOME=xdg-sigla colitur config --show --sigla-book bogus
-  config file: xdg-sigla/colitur/config.ini (not found)
-  lang             la                       (default)
-  template         (none)                   (default)
-  format           (none)                   (default)
-  sigla_style      la                       (default)
   colitur: unknown --sigla-book "bogus" (want "full" or "abbr")
   [2]
 
diff --git a/test/test_citation.ml b/test/test_citation.ml
index 133553b..dae4610 100644
--- a/test/test_citation.ml
+++ b/test/test_citation.ml
@@ -283,6 +283,11 @@ let parses input expected () =
   | Error e -> Alcotest.failf "%s did not parse: %s" input e
   | Ok t -> Alcotest.(check s) input expected (show t)
 
+let rejects input () =
+  match P.parse input with
+  | Ok t -> Alcotest.failf "%s should not parse, got %s" input (show t)
+  | Error _ -> ()
+
 let test_rejects_unknown_book () =
   Alcotest.(check bool) "error" true (Result.is_error (P.parse "Nonesuch 1:1"))
 
@@ -321,6 +326,20 @@ let parse_suite =
     ("ordinal 3 parses",  `Quick, parses "3 Kings 17:8-16" "kings_3|17:8-16");
     ("ordinal 3 dotted",  `Quick, parses "3 Kgs. 19:3-8" "kings_3|19:3-8");
     ("ordinal 4 parses",  `Quick, parses "4 Kings 5:1-15" "kings_4|5:1-15");
+    (* A citation number must be plain digits and positive. OCaml's
+       [int_of_string] also accepts its own literal syntax, so "1_1" parsed
+       as chapter ELEVEN and "+5" as 5 -- a typo silently becoming a
+       DIFFERENT chapter, which no amount of downstream care can catch.
+       Reachable through a user overlay, which supplies arbitrary strings. *)
+    ("rejects underscore in chapter", `Quick, rejects "Luke 1_1:5");
+    ("rejects underscore in verse",   `Quick, rejects "Luke 1:5_0");
+    ("rejects plus in chapter",       `Quick, rejects "Luke +1:5");
+    ("rejects plus in verse",         `Quick, rejects "Luke 1:+5");
+    ("rejects chapter zero",          `Quick, rejects "Luke 0:5");
+    ("rejects verse zero",            `Quick, rejects "Luke 1:0");
+    (* A descending range is always a transcription error. *)
+    ("rejects descending range",      `Quick, rejects "Luke 1:20-10");
+    ("allows a single-verse range",   `Quick, parses "Luke 1:5-5" "luke|1:5-5");
     ("unknown book",      `Quick, test_rejects_unknown_book);
     ("garbage",           `Quick, test_rejects_garbage) ]
 
diff --git a/test/test_lang_coverage.ml b/test/test_lang_coverage.ml
index 0271a59..d41f7e7 100644
--- a/test/test_lang_coverage.ml
+++ b/test/test_lang_coverage.ml
@@ -11,6 +11,14 @@ let la () =
   | Ok t -> t
   | Error e -> Alcotest.failf "lang/la.ini: %s" e
 
+(* en.ini resolved THROUGH la.ini, the way a real run resolves it: en
+   declares `fallback = la`, so a key it omits must reach the Latin table
+   rather than degrade to the raw key. *)
+let en () =
+  match L.of_string (read "../lang/en.ini") with
+  | Ok t -> L.with_fallback t (la ())
+  | Error e -> Alcotest.failf "lang/en.ini: %s" e
+
 (* Every slug the engine can emit -- temporal ("^ef-") AND sanctoral (a fixed
    saint's day) alike -- must have a Latin name. THIS IS THE TEST THAT WOULD
    HAVE CAUGHT THE ORIGINAL DEFECT -- a booklet printed "ef-septuagesima-
@@ -145,6 +153,112 @@ let test_no_book_name_is_an_internal_id () =
   Alcotest.(check (list string)) "no book name is its own internal id" []
     leaked
 
+(* Two DIFFERENT books must not share a name. Found by audit: `1 Cor` and
+   `2 Cor` both rendered as "Epistola ad Corinthios" under --sigla-book full,
+   and so did Thessalonians, Timothy and Peter -- 108 citations in 2027 alone
+   that a reader cannot resolve to a book. The same defect had already been
+   fixed for the two Books of Kings and simply not generalised.
+
+   Sharing a name is CORRECT, though, when the two ids are the same physical
+   book under different numbering -- `osee`/`hosea`, `jonas`/`jonah`,
+   `apocalypse`/`revelation`. A tradition maps one onto the other, so the
+   rule is exact: a shared name is a defect UNLESS some tradition relates the
+   two ids. *)
+let test_no_two_books_share_a_name () =
+  let la = la () in
+  let traditions =
+    let text =
+      let ic = open_in_bin "../lang/traditions.ini" in
+      let s = really_input_string ic (in_channel_length ic) in
+      close_in ic; s
+    in
+    match Colitur_kernel.Overlay_ini.parse_sections text with
+    | Ok ss ->
+        List.concat_map (fun (sc : Colitur_kernel.Overlay_ini.section) ->
+          sc.Colitur_kernel.Overlay_ini.fields) ss
+    | Error e -> Alcotest.failf "traditions.ini: %s" e
+  in
+  let related a b =
+    List.exists (fun (x, y) -> (x = a && y = b) || (x = b && y = a)) traditions
+  in
+  let offenders = ref [] in
+  List.iter
+    (fun form ->
+      let seen = Hashtbl.create 64 in
+      List.iter
+        (fun id ->
+          let n = Colitur_citation.Book.to_string id in
+          let key = n ^ "." ^ form in
+          let v = L.bible la key in
+          if v <> key then
+            match Hashtbl.find_opt seen v with
+            | Some other when not (related n other) ->
+                offenders := Printf.sprintf "%s: %s and %s" v other n :: !offenders
+            | _ -> Hashtbl.replace seen v n)
+        Colitur_citation.Book.all)
+    [ "full"; "abbr" ];
+  Alcotest.(check (list string)) "no two different books share a name" []
+    (List.sort compare !offenders)
+
+(* SPEC SECTION 8.5: each shipped style must parse its own rendered output.
+   It did not. Measured at the time: 32 of 52 Latin abbreviations and 49 of 52
+   Latin full titles failed to re-parse, so a user who copied a citation out
+   of `colitur readings` into an overlay handed the parser a string it could
+   not read; [Sigla.format] passed it through untouched and, say, an English
+   full-name booklet printed a Latin abbreviation with no warning. Closed by
+   registering every shipped name as a spelling and by teaching [split_book]
+   multi-word titles.
+
+   A name may resolve to a DIFFERENT id than the one it was rendered from,
+   but only when the two are the same physical book under different
+   numbering: "Sir" is registered to [ecclesiasticus] and the modern id
+   [sirach] maps onto it. The parser has no tradition context, so it returns
+   the Vulgate id, and that is right rather than tolerated. *)
+let test_shipped_styles_round_trip () =
+  let traditions =
+    let text =
+      let ic = open_in_bin "../lang/traditions.ini" in
+      let s = really_input_string ic (in_channel_length ic) in
+      close_in ic; s
+    in
+    match Colitur_kernel.Overlay_ini.parse_sections text with
+    | Ok ss ->
+        List.concat_map
+          (fun (sc : Colitur_kernel.Overlay_ini.section) ->
+            sc.Colitur_kernel.Overlay_ini.fields)
+          ss
+    | Error e -> Alcotest.failf "traditions.ini: %s" e
+  in
+  let related a b =
+    a = b || List.exists (fun (x, y) -> (x = a && y = b) || (x = b && y = a)) traditions
+  in
+  let check_file label t =
+    List.concat_map
+      (fun form ->
+        List.filter_map
+          (fun id ->
+            let n = Colitur_citation.Book.to_string id in
+            let name = L.bible t (n ^ "." ^ form) in
+            if name = n ^ "." ^ form then None
+            else
+              match Colitur_citation.Parse.parse (name ^ " 5:12-14") with
+              | Ok r
+                when related
+                       (Colitur_citation.Book.to_string r.Colitur_citation.Parse.book)
+                       n ->
+                  None
+              | Ok r ->
+                  Some
+                    (Printf.sprintf "%s %s/%s -> %s" label name form
+                       (Colitur_citation.Book.to_string r.Colitur_citation.Parse.book))
+              | Error e -> Some (Printf.sprintf "%s %s/%s: %s" label name form e))
+          Colitur_citation.Book.all)
+      [ "full"; "abbr" ]
+  in
+  let bad = check_file "la" (la ()) @ check_file "en" (en ()) in
+  Alcotest.(check (list string)) "every shipped book name re-parses" []
+    (List.sort compare bad)
+
 (* lang/en.ini is DELIBERATELY partial (see its own header note): it declares
    [meta] fallback = la, so a slug it does not carry itself should still
    resolve through the chain to la.ini's name rather than degrade to the bare
@@ -189,4 +303,8 @@ let suite =
       Alcotest.test_case "every book named, both forms" `Quick test_every_book_named;
       Alcotest.test_case "no book name is an internal id" `Quick
         test_no_book_name_is_an_internal_id;
+      Alcotest.test_case "no two books share a name" `Quick
+        test_no_two_books_share_a_name;
+      Alcotest.test_case "shipped styles round-trip" `Quick
+        test_shipped_styles_round_trip;
       Alcotest.test_case "en.ini falls back to Latin" `Quick test_en_falls_back_to_latin ] )
diff --git a/test/test_view.ml b/test/test_view.ml
index b2d302e..a542419 100644
--- a/test/test_view.ml
+++ b/test/test_view.ml
@@ -292,6 +292,25 @@ let test_vocabularies_localise () =
   Alcotest.(check string) "rite_name" "Missale Romanum, editio typica 1962"
     (as_str (get [ "rite_name" ] v))
 
+(* A month answers to BOTH spellings of its own name. The week objects nested
+   inside a month expose [month_name]/[month_num]/[month_abbr], so an author
+   who learned those names there reaches for them one level up too -- and an
+   unknown key renders as the EMPTY STRING by design, so the mistake produced
+   a silently blank heading rather than any error. Found by writing a template
+   from scratch; the shipped ones all sidestep it. *)
+let test_month_answers_to_both_spellings () =
+  let v = view_named 2027 in
+  let jan = List.hd (as_list (get [ "months" ] v)) in
+  let same a b =
+    Alcotest.(check string) (a ^ " = " ^ b) (as_str (get [ a ] jan))
+      (as_str (get [ b ] jan))
+  in
+  same "name" "month_name";
+  same "num" "month_num";
+  Alcotest.(check string) "month_name" "Ianuarius" (as_str (get [ "month_name" ] jan));
+  Alcotest.(check bool) "month_abbr is present and non-empty" true
+    (as_str (get [ "month_abbr" ] jan) <> "")
+
 let suite =
   ( "View",
     [ Alcotest.test_case "year shape" `Quick test_year_shape;