diff --git a/.gitignore b/.gitignore index aa2b191..80e90c7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ -.venv/ +.venv*/ __pycache__/ *.py[cod] .pytest_cache/ diff --git a/README.md b/README.md index 23841de..c91dd0a 100644 --- a/README.md +++ b/README.md @@ -71,9 +71,10 @@ and independently checks all of the rules that protect publication: Raw bytes remain authoritative. The adapter derives the public plain text only after verification and retains the original contract records internally. Validated entries are held in a compressed, disk-backed spool. Commentary entries -are then normalized into disk-backed chapter buckets and emitted in canonical -GetBible book/chapter order; this supports source modules whose versification orders -canonical or deuterocanonical books differently. Dictionary definitions are written +are then normalized into disk-backed chapter buckets, collapsed so that a comment +attached to a verse range is stored once rather than once per verse, and emitted in +canonical GetBible book/chapter order; this supports source modules whose versification +orders canonical or deuterocanonical books differently. Dictionary definitions are written one at a time. Book, whole-commentary, and whole-dictionary documents are streamed from the documents they contain rather than assembled in memory. This keeps memory bounded for large modules without weakening the contract or the all-or-nothing @@ -118,8 +119,7 @@ byte-for-byte. One client parser therefore handles all three: "book": 43, "chapter": 1, "verse": 1, - "name": "John 1:1", - "anchor": {"book": 43, "chapter": 1, "verse": 1, "osis": "John.1.1"}, + "osis": "John.1.1", "text": "...", "references": [{"osis": "Gen.1.1", "book": 1, "chapter": 1, "verse": 1}] } @@ -127,6 +127,46 @@ byte-for-byte. One client parser therefore handles all three: } ``` +### One comment, stored once + +A SWORD commentary attaches a comment to a verse *range*, and the extractor reports +that same text once for every verse in the range. Publishing an entry per verse +stored the identical paragraph dozens of times — Augustine's exposition of a psalm +reappeared under all 176 verses of Psalm 119, and the whole-commentary documents grew +into the hundreds of megabytes without carrying any more text. + +Each distinct comment is therefore published **once**, anchored at the lowest verse it +covers. When it covers more than one verse, `verses` lists every verse it applies to: + +```json +{ + "book": 19, + "chapter": 119, + "verse": 1, + "verses": [1, 2, 3, 4, 5, 6, 7, 8], + "osis": "Ps.119.1", + "text": "..." +} +``` + +Resolving a verse is one rule: **an entry covers `verses` when that member is present, +and `verse` alone when it is not.** + +```js +const forVerse = (chapter, n) => + chapter.entries.find(e => (e.verses ?? [e.verse]).includes(n)) +``` + +Nothing is dropped by this — every verse the source commented on still resolves to its +comment. Grouping stops at the chapter boundary, because a chapter document is the +addressable unit and has to stand alone, so a comment spanning a chapter break is +published in both chapters. + +An entry carries no `name` and no `anchor` object. Both only restated values already +present on the entry or its chapter: `name` is the book name with `chapter:verse`, and +`anchor` repeated `book`, `chapter`, and `verse` verbatim. `osis` — the source module's +own key for the anchor verse — is kept as a plain member. + Introductions are published, not discarded. A book introduction is chapter `0`, so Clarke's introduction to Daniel is `clarke/27/0.json`. A chapter introduction is verse `0`, and appears as the first entry of its own chapter document. @@ -135,6 +175,30 @@ is verse `0`, and appears as the first entry of its own chapter document. `metadata.json` reports its licence, counts, and the byte size of the whole-commentary document so a client can decide before requesting it. +`metadata.json` also carries a `storage` block, which is the build's own measurement +of this module rather than anything a client needs: + +```json +{ + "source_entry_count": 168447, + "source_text_bytes": 402653184, + "text_bytes": 41943040, + "repetition_ratio": 9.6, + "chapter_bytes": 44040192, + "book_bytes": 44564480, + "commentary_bytes": 45088768, + "published_bytes": 133693440 +} +``` + +`repetition_ratio` is how many times the average byte of source text was repeated +across the verse range it was attached to — the multiplier the collapse removes. The +three `*_bytes` members are what each level of the API costs on disk. + +No generated document may exceed `--max-document-bytes` (default 95 MB, just under the +100 MB a Git remote refuses). The build fails and names the file rather than producing +a tree that is rejected at push time, hours later. Set it to `0` to disable the check. + ## Dictionary API ```text diff --git a/docs/deployment.md b/docs/deployment.md index bba9a4c..b1945eb 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -30,6 +30,9 @@ the deployment stops being safe: 4. **Every document is plain text JSON.** No HTML is published anywhere, which is why the origin can send `Content-Security-Policy: default-src 'none'` and why no consumer has to sanitize a response. +5. **No document exceeds `--max-document-bytes`** (95 MB by default). The build + fails naming the offending file rather than publishing a tree the publication + remote would reject, so the origin never sees a half-pushed corpus. ## Server layout diff --git a/schemas/commentary-chapter.schema.json b/schemas/commentary-chapter.schema.json index b63391b..0b06329 100644 --- a/schemas/commentary-chapter.schema.json +++ b/schemas/commentary-chapter.schema.json @@ -21,22 +21,27 @@ "$defs": { "entry": { "type": "object", - "required": ["book", "chapter", "verse", "name", "anchor", "text"], + "description": "One comment. A source module attaches a comment to a verse range and repeats it for every verse in that range; it is published once here, anchored at the lowest verse it covers, with `verses` listing every verse it applies to when that is more than one.", + "required": ["book", "chapter", "verse", "text"], "properties": { "book": {"type": "integer", "minimum": 1, "maximum": 83}, "chapter": {"type": "integer", "minimum": 0}, - "verse": {"type": "integer", "minimum": 0}, - "name": {"type": "string"}, - "anchor": { - "type": "object", - "required": ["book", "chapter", "verse"], - "properties": { - "book": {"type": "integer", "minimum": 1, "maximum": 83}, - "chapter": {"type": "integer", "minimum": 0}, - "verse": {"type": "integer", "minimum": 0}, - "osis": {"type": "string"} - }, - "additionalProperties": false + "verse": { + "type": "integer", + "minimum": 0, + "description": "The lowest verse this comment covers." + }, + "verses": { + "type": "array", + "description": "Every verse this comment covers, including `verse`. Absent when it covers only `verse`.", + "items": {"type": "integer", "minimum": 0}, + "minItems": 2, + "uniqueItems": true + }, + "osis": { + "type": "string", + "minLength": 1, + "description": "The source module's own key for the anchor verse." }, "text": {"type": "string"}, "references": {"$ref": "#/$defs/references"} diff --git a/scripts/validate_build.py b/scripts/validate_build.py index adb7758..d14bea1 100644 --- a/scripts/validate_build.py +++ b/scripts/validate_build.py @@ -30,6 +30,24 @@ def _assert_composed(composed: list[Any], parts: list[Path], where: str) -> None raise RuntimeError(f"{where} does not match the document served at {path}") +def _assert_no_repeated_text(chapter: dict[str, Any], where: Path) -> None: + """The whole point of the collapse: one comment is stored once, not once per verse. + + Two distinct comments may still land on the same verse — a source module can emit + more than one record for a verse — so verse coverage is deliberately not asserted + to be disjoint. What must hold is that no text repeats. + """ + seen: set[str] = set() + for entry in chapter["entries"]: + text = entry["text"] + if text in seen: + raise RuntimeError(f"{where} publishes the same comment more than once") + seen.add(text) + verses = entry.get("verses", [entry["verse"]]) + if entry["verse"] != min(verses): + raise RuntimeError(f"{where} anchors an entry above the lowest verse it covers") + + def validate_commentary(root: Path, complete_path: Path) -> dict[str, Any]: metadata = read_json(root / "metadata.json") books = read_json(root / "books.json") @@ -54,8 +72,9 @@ def validate_commentary(root: Path, complete_path: Path) -> dict[str, Any]: if chapter.get("schema") != "getbible-commentary-chapter-v1" or not chapter.get("entries"): raise RuntimeError("Commentary chapter produced no entries") first = chapter["entries"][0] - if not all(name in first for name in ("book", "chapter", "verse", "anchor", "text")): + if not all(name in first for name in ("book", "chapter", "verse", "text")): raise RuntimeError("Commentary entry is not linked to a Bible API coordinate") + _assert_no_repeated_text(chapter, chapter_paths[0]) _reject_markup(chapter, "chapter") complete = read_json(complete_path) diff --git a/src/study_builder/cli.py b/src/study_builder/cli.py index 6f91e1f..3af7726 100644 --- a/src/study_builder/cli.py +++ b/src/study_builder/cli.py @@ -11,7 +11,7 @@ from study_builder.http import HttpClient from study_builder.pipeline import BuildPipeline, PipelineConfig from study_builder.policy import ModulePolicy -from study_builder.util import reset_directory +from study_builder.util import DOCUMENT_CEILING_BYTES, reset_directory def repository_root() -> Path: @@ -65,6 +65,14 @@ def parser() -> argparse.ArgumentParser: "git@github.com:getbible/dictionaries.git", ), ) + build.add_argument( + "--max-document-bytes", + type=int, + default=int( + os.environ.get("STUDY_BUILDER_MAX_DOCUMENT_BYTES", DOCUMENT_CEILING_BYTES) or 0 + ), + help="Fail the build rather than publish a document above this size (0 disables)", + ) build.add_argument("--commentaries-branch", default="main") build.add_argument("--dictionaries-branch", default="main") build.add_argument("--pull", action="store_true", help="Clone/pull target repositories") @@ -122,6 +130,7 @@ def _build(args: argparse.Namespace) -> int: dictionaries_repo=args.dictionaries_repo, commentaries_branch=args.commentaries_branch, dictionaries_branch=args.dictionaries_branch, + max_document_bytes=args.max_document_bytes, ) report = BuildPipeline(config).run() print(json.dumps(report.as_dict(), ensure_ascii=False, indent=2)) diff --git a/src/study_builder/commentaries.py b/src/study_builder/commentaries.py index 517e29a..3974579 100644 --- a/src/study_builder/commentaries.py +++ b/src/study_builder/commentaries.py @@ -12,7 +12,14 @@ from study_builder.chapter_spool import CommentaryChapterSpool from study_builder.content import extract_osis_references, public_content from study_builder.models import ModuleDescriptor, NativeExport -from study_builder.util import read_json, slug, write_composed_json, write_json +from study_builder.util import ( + DOCUMENT_CEILING_BYTES, + enforce_document_ceiling, + read_json, + slug, + write_composed_json, + write_json, +) # Reserved inside a commentary directory; an entry may never claim these names. RESERVED_DOCUMENTS = {"metadata.json", "books.json"} @@ -26,9 +33,16 @@ class CommentaryWriter: at its own path and a client needs only one parser for all three levels. """ - def __init__(self, root: Path, books: BookRegistry, schemas_dir: Path) -> None: + def __init__( + self, + root: Path, + books: BookRegistry, + schemas_dir: Path, + max_document_bytes: int = DOCUMENT_CEILING_BYTES, + ) -> None: self.root = root self.books = books + self.max_document_bytes = max_document_bytes self.chapter_schema = read_json(schemas_dir / "commentary-chapter.schema.json") def write(self, module: ModuleDescriptor, exported: NativeExport) -> tuple[dict, dict]: @@ -37,11 +51,19 @@ def write(self, module: ModuleDescriptor, exported: NativeExport) -> tuple[dict, chapter_files: dict[int, list[Path]] = defaultdict(list) chapter_counts: dict[int, list[tuple[int, int]]] = defaultdict(list) entry_count = 0 + # Measured, not assumed: what the source offered against what is published. + source_entry_count = 0 + source_text_bytes = 0 + text_bytes = 0 + chapter_bytes = 0 + book_bytes = 0 with CommentaryChapterSpool() as chapters: for source in exported.entries: entry = self._entry(source) if entry is not None: + source_entry_count += 1 + source_text_bytes += len(entry["text"].encode("utf-8")) chapters.append(entry) for book_number, chapter_number in chapters.coordinates(): @@ -61,9 +83,11 @@ def write(self, module: ModuleDescriptor, exported: NativeExport) -> tuple[dict, validate(document, self.chapter_schema) path = module_root / str(book_number) / f"{chapter_number}.json" write_json(path, document) + chapter_bytes += enforce_document_ceiling(path, self.max_document_bytes) chapter_files[book_number].append(path) chapter_counts[book_number].append((chapter_number, len(chapter_entries))) entry_count += len(chapter_entries) + text_bytes += sum(len(item["text"].encode("utf-8")) for item in chapter_entries) book_files: list[Path] = [] books_index: list[dict[str, Any]] = [] @@ -84,6 +108,7 @@ def write(self, module: ModuleDescriptor, exported: NativeExport) -> tuple[dict, "chapters", chapter_files[book_number], ) + book_bytes += enforce_document_ceiling(path, self.max_document_bytes) book_files.append(path) books_index.append( { @@ -120,10 +145,21 @@ def write(self, module: ModuleDescriptor, exported: NativeExport) -> tuple[dict, "books", book_files, ) + complete_bytes = enforce_document_ceiling(complete, self.max_document_bytes) chapter_count = sum(len(records) for records in chapter_counts.values()) + storage = { + "source_entry_count": source_entry_count, + "source_text_bytes": source_text_bytes, + "text_bytes": text_bytes, + "repetition_ratio": round(source_text_bytes / text_bytes, 3) if text_bytes else 1.0, + "chapter_bytes": chapter_bytes, + "book_bytes": book_bytes, + "commentary_bytes": complete_bytes, + "published_bytes": chapter_bytes + book_bytes + complete_bytes, + } metadata = self._metadata( - module, module_id, len(books_index), chapter_count, entry_count, complete.stat().st_size + module, module_id, len(books_index), chapter_count, entry_count, storage ) write_json(module_root / "metadata.json", metadata) record = { @@ -154,27 +190,15 @@ def _entry(self, source: dict[str, Any]) -> dict[str, Any] | None: content = public_content(source) if not content["text"]: return None - label = book.name - if chapter: - label += f" {chapter}" - if verse_number: - label += f":{verse_number}" - anchor: dict[str, Any] = { + entry: dict[str, Any] = { "book": book.number, "chapter": chapter, "verse": verse_number, } osis = str(verse.get("osis", "")) if osis: - anchor["osis"] = osis - entry: dict[str, Any] = { - "book": book.number, - "chapter": chapter, - "verse": verse_number, - "name": label, - "anchor": anchor, - **content, - } + entry["osis"] = osis + entry.update(content) related = [] for reference in extract_osis_references( str(source.get("raw", "")), str(source.get("html", "")) @@ -190,15 +214,67 @@ def _entry(self, source: dict[str, Any]) -> dict[str, Any] | None: def _chapter_entries( chapters: CommentaryChapterSpool, book_number: int, chapter_number: int ) -> list[dict[str, Any]]: - seen: set[tuple[int, str]] = set() - collected: list[dict[str, Any]] = [] + """Publish each distinct comment once, listing every verse it covers. + + A SWORD commentary attaches one comment to a verse *range* and the extractor + reports that same text once per verse in the range, so writing an entry per + verse stored the identical paragraph dozens of times — Augustine's exposition + of a psalm reappeared for all 176 verses of Psalm 119. Nothing is dropped: + the text is published once, anchored at the lowest verse it covers, and + `verses` names the rest, so every verse still resolves to its comment. + + Grouping stops at the chapter boundary. A chapter document is the addressable + unit and has to stand alone, so a comment spanning two chapters is published + in each of them rather than only in the first. + """ + order: list[str] = [] + anchors: dict[str, dict[str, Any]] = {} + covered: dict[str, set[int]] = {} + references: dict[str, dict[tuple[Any, ...], dict[str, Any]]] = {} for entry in chapters.entries(book_number, chapter_number): - unique = (int(entry["verse"]), str(entry.get("text", ""))) - if unique in seen: - continue - seen.add(unique) - collected.append(entry) - collected.sort(key=lambda item: (item["verse"], item["name"])) + text = str(entry.get("text", "")) + verse = int(entry["verse"]) + if text not in anchors: + order.append(text) + anchors[text] = entry + covered[text] = set() + references[text] = {} + elif verse < int(anchors[text]["verse"]): + # The lowest verse anchors the published entry, so its `osis` is the + # one the source module keyed the comment on. + anchors[text] = entry + covered[text].add(verse) + # A repeated comment repeats its references; union them so a range that + # does differ verse to verse keeps every reference it carried. + for reference in entry.get("references", ()): + references[text][ + ( + reference.get("osis", ""), + reference.get("book", 0), + reference.get("chapter", 0), + reference.get("verse", 0), + ) + ] = reference + + collected: list[dict[str, Any]] = [] + for text in order: + anchor = anchors[text] + verses = sorted(covered[text]) + published: dict[str, Any] = { + "book": int(anchor["book"]), + "chapter": int(anchor["chapter"]), + "verse": verses[0], + } + if len(verses) > 1: + published["verses"] = verses + if anchor.get("osis"): + published["osis"] = str(anchor["osis"]) + published["text"] = text + related = [reference for _, reference in sorted(references[text].items())] + if related: + published["references"] = related + collected.append(published) + collected.sort(key=lambda item: (item["verse"], item["text"])) return collected @staticmethod @@ -208,7 +284,7 @@ def _metadata( book_count: int, chapter_count: int, entry_count: int, - complete_bytes: int, + storage: dict[str, Any], ) -> dict[str, Any]: return { "schema": "getbible-commentary-metadata-v1", @@ -224,7 +300,8 @@ def _metadata( "book_count": book_count, "chapter_count": chapter_count, "entry_count": entry_count, - "bytes": complete_bytes, + "bytes": storage["commentary_bytes"], + "storage": storage, "books_url": "books.json", "book_url_template": "{book}.json", "chapter_url_template": "{book}/{chapter}.json", diff --git a/src/study_builder/dictionaries.py b/src/study_builder/dictionaries.py index 19ea69d..7557a9d 100644 --- a/src/study_builder/dictionaries.py +++ b/src/study_builder/dictionaries.py @@ -16,7 +16,14 @@ from study_builder.books import BookRegistry from study_builder.content import extract_osis_references, public_content from study_builder.models import ModuleDescriptor, NativeExport -from study_builder.util import read_json, slug, write_composed_json, write_json +from study_builder.util import ( + DOCUMENT_CEILING_BYTES, + enforce_document_ceiling, + read_json, + slug, + write_composed_json, + write_json, +) _STRONG_KEY = re.compile(r"^(?:strong:)?([GH])?0*(\d{1,5})(?:!.*)?$", re.IGNORECASE) _SAFE_ENTRY_KEY = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,119}$") @@ -126,9 +133,16 @@ class DictionaryWriter: key has an identifier, so a single pass cannot produce a navigable link graph. """ - def __init__(self, root: Path, books: BookRegistry, schemas_dir: Path) -> None: + def __init__( + self, + root: Path, + books: BookRegistry, + schemas_dir: Path, + max_document_bytes: int = DOCUMENT_CEILING_BYTES, + ) -> None: self.root = root self.books = books + self.max_document_bytes = max_document_bytes self.schema = read_json(schemas_dir / "dictionary-entry.schema.json") def write(self, module: ModuleDescriptor, exported: NativeExport) -> tuple[dict, dict]: @@ -186,9 +200,10 @@ def write(self, module: ModuleDescriptor, exported: NativeExport) -> tuple[dict, "entries", [entry_files[item.entry_id] for item in index], ) + complete_bytes = enforce_document_ceiling(complete, self.max_document_bytes) metadata = self._metadata( - module, module_id, prefix, len(staged), unique_keys, complete.stat().st_size + module, module_id, prefix, len(staged), unique_keys, complete_bytes ) write_json(module_root / "metadata.json", metadata) record = { diff --git a/src/study_builder/models.py b/src/study_builder/models.py index 8024eb0..2f409a4 100644 --- a/src/study_builder/models.py +++ b/src/study_builder/models.py @@ -75,6 +75,8 @@ class BuildReport: failed: list[dict[str, str]] = field(default_factory=list) diagnostics: dict[str, list[dict[str, Any]]] = field(default_factory=dict) commits: dict[str, str | None] = field(default_factory=dict) + # Measured per module: how much the source repeated, and what each level costs. + storage: dict[str, dict[str, Any]] = field(default_factory=dict) def as_dict(self) -> dict[str, Any]: return { @@ -86,6 +88,7 @@ def as_dict(self) -> dict[str, Any]: "skipped": self.skipped, "failed": self.failed, "diagnostics": self.diagnostics, + "storage": self.storage, "commits": self.commits, } diff --git a/src/study_builder/pipeline.py b/src/study_builder/pipeline.py index dc66f8d..10837b6 100644 --- a/src/study_builder/pipeline.py +++ b/src/study_builder/pipeline.py @@ -19,6 +19,7 @@ from study_builder.native import SwordExporter from study_builder.policy import ModulePolicy from study_builder.util import ( + DOCUMENT_CEILING_BYTES, hash_tree, replace_tree, reset_directory, @@ -77,6 +78,7 @@ class PipelineConfig: dictionaries_repo: str = "git@github.com:getbible/dictionaries.git" commentaries_branch: str = "main" dictionaries_branch: str = "main" + max_document_bytes: int = DOCUMENT_CEILING_BYTES class BuildPipeline: @@ -200,15 +202,30 @@ def run(self) -> BuildReport: for item in exported.diagnostics ] writer = ( - CommentaryWriter(generated_roots[kind], self.books, self.config.schemas_dir) + CommentaryWriter( + generated_roots[kind], + self.books, + self.config.schemas_dir, + self.config.max_document_bytes, + ) if kind == "commentaries" else DictionaryWriter( - generated_roots[kind], self.books, self.config.schemas_dir + generated_roots[kind], + self.books, + self.config.schemas_dir, + self.config.max_document_bytes, ) ) - record, _ = writer.write(module, exported) + record, metadata = writer.write(module, exported) summaries[kind].append(record) report.built[kind].append(module.name) + if metadata.get("storage"): + report.storage[module.name] = { + "resource": kind, + "id": record["id"], + "entry_count": record["entry_count"], + **metadata["storage"], + } except Exception as error: LOG.exception("Failed to build %s", module.name) report.failed.append( diff --git a/src/study_builder/util.py b/src/study_builder/util.py index 09dc25f..cea8b9f 100644 --- a/src/study_builder/util.py +++ b/src/study_builder/util.py @@ -2,6 +2,7 @@ import hashlib import json +import logging import os import re import shutil @@ -11,8 +12,20 @@ from pathlib import Path from typing import IO, Any +LOG = logging.getLogger(__name__) + _SAFE_SLUG = re.compile(r"[^a-z0-9._-]+") +# A Git remote rejects a blob above 100 MB outright and warns above 50 MB. The +# builder holds itself below both, so an oversized document is caught here — in +# the build, naming the file — rather than hours later in a rejected push. +DOCUMENT_WARNING_BYTES = 50 * 1024 * 1024 +DOCUMENT_CEILING_BYTES = 95 * 1024 * 1024 + + +class DocumentTooLarge(RuntimeError): + """A generated document exceeds the size a Git remote will accept.""" + def utc_now() -> str: return datetime.now(UTC).replace(microsecond=0).isoformat().replace("+00:00", "Z") @@ -76,9 +89,33 @@ def _write_indented(handle: IO[str], source: Path, prefix: str) -> None: handle.write(prefix + line if line else "") +def megabytes(size: int) -> str: + return f"{size / 1024 / 1024:.2f} MB" + + +def enforce_document_ceiling(path: Path, ceiling: int = DOCUMENT_CEILING_BYTES) -> int: + """Refuse to publish a document a Git remote would reject, and say which one.""" + size = path.stat().st_size + if ceiling and size > ceiling: + raise DocumentTooLarge( + f"{path.name} is {megabytes(size)}, above the {megabytes(ceiling)} document " + "ceiling. Publishing it would be rejected by the remote. Either the source " + "module grew beyond what a single document can carry, or it is repeating " + "content that should have been collapsed." + ) + if size > DOCUMENT_WARNING_BYTES: + LOG.warning( + "%s is %s, above the %s a Git remote warns about", + path.name, + megabytes(size), + megabytes(DOCUMENT_WARNING_BYTES), + ) + return size + + def write_composed_json( path: Path, header: dict[str, Any], member: str, sources: Sequence[Path] -) -> None: +) -> int: """Write an envelope whose array member embeds already-written documents. The members are streamed from disk rather than held in memory, and each one is @@ -104,6 +141,7 @@ def write_composed_json( handle.write("]\n}\n") temporary = Path(handle.name) os.replace(temporary, path) + return path.stat().st_size def replace_tree(source: Path, destination: Path) -> None: diff --git a/tests/test_commentaries.py b/tests/test_commentaries.py index e4f533a..21e1563 100644 --- a/tests/test_commentaries.py +++ b/tests/test_commentaries.py @@ -1,8 +1,11 @@ import json +import pytest + from study_builder.books import BookRegistry from study_builder.commentaries import CommentaryWriter from study_builder.models import NativeExport +from study_builder.util import DocumentTooLarge class OnePassEntries: @@ -39,11 +42,12 @@ def entry(osis, book, chapter, verse, text, *, raw=None, html=""): } -def write(tmp_path, project_root, module, entries): +def write(tmp_path, project_root, module, entries, max_document_bytes=0): writer = CommentaryWriter( tmp_path, BookRegistry(project_root / "conf/book_registry.json"), project_root / "schemas", + max_document_bytes, ) return writer.write(module, NativeExport(metadata={"record_type": "module"}, entries=entries)) @@ -71,7 +75,7 @@ def test_commentary_matches_v3_book_chapter_verse_contract( assert metadata["chapter_url_template"] == "{book}/{chapter}.json" assert (chapter["book"], chapter["chapter"]) == (43, 1) assert chapter["entries"][0]["verse"] == 1 - assert chapter["entries"][0]["anchor"]["osis"] == "John.1.1" + assert chapter["entries"][0]["osis"] == "John.1.1" assert chapter["entries"][0]["references"][0]["book"] == 43 assert record["entry_count"] == 1 @@ -107,12 +111,12 @@ def test_commentary_publishes_book_and_chapter_introductions( ) introduction = json.loads((tmp_path / "testcom/27/0.json").read_text(encoding="utf-8")) assert introduction["chapter"] == 0 - assert introduction["entries"][0]["name"] == "Daniel" + assert introduction["name"] == "Daniel" assert introduction["entries"][0]["text"] == "About the book of Daniel." chapter = json.loads((tmp_path / "testcom/27/1.json").read_text(encoding="utf-8")) assert [item["verse"] for item in chapter["entries"]] == [0, 1] - assert chapter["entries"][0]["name"] == "Daniel 1" + assert chapter["name"] == "Daniel" books = json.loads((tmp_path / "testcom/books.json").read_text(encoding="utf-8")) assert books["books"][0]["chapters"] == [0, 1] @@ -177,6 +181,106 @@ def test_commentary_consumes_source_entries_once(tmp_path, project_root, comment assert (tmp_path / "testcom/1/2.json").is_file() +def test_range_comment_is_stored_once_and_lists_every_verse_it_covers( + tmp_path, project_root, commentary_module +) -> None: + """A SWORD range comment arrives once per verse; it must be published once.""" + exposition = "One exposition covering the whole passage." + record, metadata = write( + tmp_path, + project_root, + commentary_module, + [entry(f"Ps.119.{number}", 19, 119, number, exposition) for number in range(1, 9)], + ) + chapter = json.loads((tmp_path / "testcom/19/119.json").read_text(encoding="utf-8")) + + assert len(chapter["entries"]) == 1 + published = chapter["entries"][0] + assert published["text"] == exposition + assert published["verse"] == 1 + assert published["verses"] == [1, 2, 3, 4, 5, 6, 7, 8] + assert published["osis"] == "Ps.119.1" + assert record["entry_count"] == 1 + + # The document holds the text once, not once per verse it covers. + assert (tmp_path / "testcom/19/119.json").read_text(encoding="utf-8").count(exposition) == 1 + assert metadata["storage"]["source_entry_count"] == 8 + assert metadata["storage"]["repetition_ratio"] == 8.0 + + +def test_distinct_comments_in_one_chapter_are_all_kept( + tmp_path, project_root, commentary_module +) -> None: + write( + tmp_path, + project_root, + commentary_module, + [ + entry("Gen.1.1", 1, 1, 1, "On verse one."), + entry("Gen.1.2", 1, 1, 2, "On verse two."), + entry("Gen.1.3", 1, 1, 3, "On verse three."), + ], + ) + chapter = json.loads((tmp_path / "testcom/1/1.json").read_text(encoding="utf-8")) + assert [item["verse"] for item in chapter["entries"]] == [1, 2, 3] + assert all("verses" not in item for item in chapter["entries"]) + + +def test_a_comment_spanning_two_chapters_stays_in_both( + tmp_path, project_root, commentary_module +) -> None: + """Chapter documents must stand alone, so the collapse stops at the boundary.""" + shared = "A comment that spans the chapter break." + write( + tmp_path, + project_root, + commentary_module, + [ + entry("Gen.1.31", 1, 1, 31, shared), + entry("Gen.2.1", 1, 2, 1, shared), + ], + ) + first = json.loads((tmp_path / "testcom/1/1.json").read_text(encoding="utf-8")) + second = json.loads((tmp_path / "testcom/1/2.json").read_text(encoding="utf-8")) + assert first["entries"][0]["text"] == shared + assert second["entries"][0]["text"] == shared + + +def test_collapsed_entry_keeps_every_reference_from_the_range( + tmp_path, project_root, commentary_module +) -> None: + shared = "A comment on the passage." + write( + tmp_path, + project_root, + commentary_module, + [ + entry("Gen.1.1", 1, 1, 1, shared, raw="see John.3.16"), + entry("Gen.1.2", 1, 1, 2, shared, raw="see Rom.5.8"), + ], + ) + chapter = json.loads((tmp_path / "testcom/1/1.json").read_text(encoding="utf-8")) + published = chapter["entries"][0] + assert published["verses"] == [1, 2] + assert {item["osis"] for item in published["references"]} == {"John.3.16", "Rom.5.8"} + + +def test_a_document_above_the_ceiling_fails_the_build_naming_the_file( + tmp_path, project_root, commentary_module +) -> None: + """Caught here, in the build, rather than hours later in a rejected push.""" + with pytest.raises(DocumentTooLarge) as error: + write( + tmp_path, + project_root, + commentary_module, + [entry("Gen.1.1", 1, 1, 1, "x" * 4096)], + max_document_bytes=1024, + ) + assert "1.json" in str(error.value) + assert "ceiling" in str(error.value) + + def test_commentary_skips_entries_without_a_bible_coordinate( tmp_path, project_root, commentary_module ) -> None: