From b5049cfb5d8b79fd1f0abc755c80821985741cf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?eW=C9=98yn?= <5607939+Llewellynvdm@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:44:13 +0000 Subject: [PATCH] Store a range comment once instead of once per verse A SWORD commentary attaches one comment to a verse range, and the extractor reports that same text once for every verse in the range. Writing an entry per verse stored the identical paragraph dozens of times: an exposition of a psalm reappeared under all 176 verses of Psalm 119, and the whole-commentary documents reached 481 MB for Matthew Henry and 306 MB for FreAug without carrying any more text than the source held. That is what the publication push was rejected for. Each distinct comment is now published once, anchored at the lowest verse it covers, with `verses` listing every verse it applies to when that is more than one. Nothing is dropped: every verse the source commented on still resolves to its comment, and resolving one is a single rule -- an entry covers `verses` when that member is present and `verse` alone when it is not. Grouping stops at the chapter boundary so a chapter document still stands alone, and references from every verse in a range are unioned onto the published entry rather than taken from whichever verse happened to be seen first. Entries also lose `name` and `anchor`. Both only restated values already present on the entry or its chapter: `name` was the book name with `chapter:verse`, and `anchor` repeated `book`, `chapter`, and `verse` verbatim. `osis` stays, as a plain member, because it is the source module's own key rather than a restatement. The build now measures itself instead of leaving this to be guessed at. metadata.json carries a `storage` block reporting source against published bytes, the repetition ratio the collapse removed, and what each of the three levels costs; the build report collects the same per module, so a CI run reports it. No generated document may exceed --max-document-bytes, 95 MB by default and just under the 100 MB a Git remote refuses. The build now fails naming the offending file rather than producing a tree that is rejected at push time hours later. --- .gitignore | 2 +- README.md | 74 +++++++++++++- docs/deployment.md | 3 + schemas/commentary-chapter.schema.json | 31 +++--- scripts/validate_build.py | 21 +++- src/study_builder/cli.py | 11 +- src/study_builder/commentaries.py | 133 +++++++++++++++++++------ src/study_builder/dictionaries.py | 21 +++- src/study_builder/models.py | 3 + src/study_builder/pipeline.py | 23 ++++- src/study_builder/util.py | 40 +++++++- tests/test_commentaries.py | 112 ++++++++++++++++++++- 12 files changed, 414 insertions(+), 60 deletions(-) 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: