Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion .github/scripts/patch_fan_lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ read_schema() {
printf '%s' "$v"
}

# Why an anchor shipped no patch has to outlive its own subshell: the driver's
# final "no patch at all" check must tell a size-guard skip (still a
# publishable full-only release) from a broken fan.
SKIP_DIR="$RUNNER_TEMP/patch-fan-skips"
rm -rf "$SKIP_DIR"
mkdir -p "$SKIP_DIR"
record_skip() { # <oversized|structural> <target-version> <reason>
printf 'anchor v%s: %s\n' "$2" "$3" > "$SKIP_DIR/v$2.$1"
}

# One anchor, start to finish: pre-check, reconstitute, produce,
# verify, clean up. Every path it touches is scoped to its own offset
# so two of these can run side by side, it writes to its own log, and
Expand All @@ -53,7 +63,7 @@ produce_anchor() { # <offset> <target-version> <tag>
local PREV_DB="$ANCHOR_DIR/seforim.db"
local PATCH_OUT="$PWD/patches/patch-v${TARGET_VER}-v${THIS_VER}.db"
local PRECHECK WAIT_BUDGET PREFETCH_STATE PREFETCH_WAITED
local PREV_SCHEMA PRODUCE_RC REASON
local PREV_SCHEMA PRODUCE_RC REASON SKIP_KIND
local T_START T_DOWNLOADED T_EXTRACTED T_DONE
echo "=== Producing patch v${TARGET_VER} → v${THIS_VER} (offset $OFFSET, tag=$TAG) ==="

Expand Down Expand Up @@ -95,6 +105,7 @@ produce_anchor() { # <offset> <target-version> <tag>
echo "pre-check: $PRECHECK"
if [ "${PRECHECK%% *}" = UNPATCHABLE ]; then
echo "::warning::anchor v${TARGET_VER} ($TAG): ${PRECHECK#* } — pre-download schema check declared the anchor unpatchable; skip anchor"
record_skip structural "$TARGET_VER" "${PRECHECK#* }"
return 0
fi

Expand Down Expand Up @@ -175,6 +186,7 @@ produce_anchor() { # <offset> <target-version> <tag>
echo "schema $PREV_SCHEMA → $THIS_SCHEMA — producing the supported cross-schema delta"
else
echo "schema $PREV_SCHEMA → $THIS_SCHEMA is unsupported — skip anchor"
record_skip structural "$TARGET_VER" "schema $PREV_SCHEMA → $THIS_SCHEMA is unsupported"
rm -rf "$ANCHOR_DIR"
return 0
fi
Expand Down Expand Up @@ -231,6 +243,11 @@ produce_anchor() { # <offset> <target-version> <tag>
if [ "$PRODUCE_RC" -eq 3 ] || [ -f "$PATCH_OUT.unpatchable" ]; then
REASON=$(cat "$PATCH_OUT.unpatchable" 2>/dev/null || true)
echo "::warning::anchor v${TARGET_VER} ($TAG): ${REASON:-see PatchPipelineCli output} — producer declared the anchor unpatchable; skip anchor"
# PatchSizeGuard.MARKER_REASON_TOKEN leads the marker when the
# delta was merely too big — a full-only release, not a defect.
SKIP_KIND=structural
case "$REASON" in oversized-delta:*) SKIP_KIND=oversized ;; esac
record_skip "$SKIP_KIND" "$TARGET_VER" "${REASON:-see PatchPipelineCli output}"
# Leave nothing behind: the marker plus the producer's half-built
# .tmp (and any stale .db) must not clutter patches/, which the
# release staging step globs.
Expand Down
49 changes: 49 additions & 0 deletions .github/scripts/test_manual_release_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@
/ "io" / "github" / "kdroidfilter" / "seforimlibrary" / "common" / "patch"
/ "PatchPipelineCli.kt"
)
PATCH_SIZE_GUARD = (
Path(__file__).parents[2]
/ "generator/common/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary"
/ "common/patch/PatchSizeGuard.kt"
)


class ManualReleaseWorkflowContractTest(unittest.TestCase):
Expand Down Expand Up @@ -278,6 +283,50 @@ def test_patch_fan_skips_only_anchors_the_producer_declares_unpatchable(self):
patch_fan.index("patch fan produced no patch although prior releases exist"),
)

def test_a_corpus_wide_size_guard_skip_still_publishes_a_full_only_release(self):
# A build that churns every line legitimately loses every anchor to the
# delta size guard (Otzaria issue #1211). That is a degraded release —
# seforim.db.zst + buildstate, no patches — not a broken patch contract.
patch_fan = self.step("Produce + verify patch fan")
# produce_anchor lives in patch_fan_lib.sh, sourced into the step's shell.
fan_lib = self.fan_lib

# The kind of every skip outlives its subshell as a file, because the
# anchors run in background subshells that cannot share a variable.
self.assertIn('SKIP_DIR="$RUNNER_TEMP/patch-fan-skips"', fan_lib)
self.assertIn("record_skip() {", fan_lib)
# Every skip path records, so "no patch and no recorded skip" stays an error.
self.assertEqual(fan_lib.count("record_skip "), 3)
self.assertIn('record_skip structural "$TARGET_VER" "${PRECHECK#* }"', fan_lib)
self.assertIn('record_skip "$SKIP_KIND" "$TARGET_VER"', fan_lib)

# The oversized/structural split is driven by the token PatchSizeGuard
# puts at the head of its marker — the two must not drift apart.
token = re.search(
r'MARKER_REASON_TOKEN: String = "([^"]+)"',
PATCH_SIZE_GUARD.read_text(encoding="utf-8"),
)
self.assertIsNotNone(token, "PatchSizeGuard must expose MARKER_REASON_TOKEN")
self.assertIn(
f'case "$REASON" in {token.group(1)}:*) SKIP_KIND=oversized ;; esac',
fan_lib,
)

# No patch + at least one size-guard skip: warn and carry on.
warn_at = patch_fan.index("::warning::patch fan produced no patch:")
err_at = patch_fan.index("patch fan produced no patch although prior releases exist")
self.assertLess(warn_at, err_at)
self.assertIn('if [ "$OVERSIZED_SKIPS" -gt 0 ]; then', patch_fan)
self.assertNotIn("exit 1", patch_fan[warn_at:err_at])
# No patch and no size-guard skip is still a hard failure.
self.assertIn("exit 1", patch_fan[err_at:])

# Staging a release with zero patches is already guarded.
self.assertIn(
'if compgen -G "patches/patch-*.db.zst" > /dev/null; then',
self.step("Stage release assets"),
)

def test_patch_fan_decides_unpatchable_anchors_before_downloading_them(self):
# An anchor the producer will reject costs 110–135 s of download plus a
# decompress before anyone learns that (run 33865604251, anchor v10).
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/delta-pipeline-dryrun.yml
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,14 @@ jobs:
# catalog.pb is no longer produced; drop any stale copy so it can't
# be auto-embedded into the patch via the default -PcatalogPb path.
rm -f build/catalog.pb
# These runs test the delta MECHANICS on toy DBs, where a patch is
# legitimately huge relative to the DB — the size guard must not fire.
./gradlew :generator-common:producePatchAndVerify \
-PprevDb=$PWD/build/seforim.db.v1 \
-PnewDb=$PWD/build/seforim.db \
-Pout=$PWD/build/patch-v1-v2.db \
-PfromVersion=1 -PtoVersion=2 \
-PmaxDeltaUncompressedRatio=1000 \
--no-daemon --stacktrace

# ─── PATCH SIZE + MANIFEST SUMMARY ──────────────────────────────────────
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/delta-real-diff-arm.yml
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,14 @@ jobs:
# catalog.pb is no longer produced; drop any stale copy so it can't
# be auto-embedded into the patch via the default -PcatalogPb path.
rm -f build/catalog.pb
# These runs test the delta MECHANICS on toy DBs, where a patch is
# legitimately huge relative to the DB — the size guard must not fire.
./gradlew :generator-common:producePatchAndVerify \
-PprevDb=$PWD/build/seforim.db.v1 \
-PnewDb=$PWD/build/seforim.db \
-Pout=$PWD/build/patch-v1-v2.db \
-PfromVersion=1 -PtoVersion=2 \
-PmaxDeltaUncompressedRatio=1000 \
--no-daemon --stacktrace

- name: Summarise patch
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/delta-real-diff-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,14 @@ jobs:
# catalog.pb is no longer produced; drop any stale copy so it can't
# be auto-embedded into the patch via the default -PcatalogPb path.
rm -f build/catalog.pb
# These runs test the delta MECHANICS on toy DBs, where a patch is
# legitimately huge relative to the DB — the size guard must not fire.
./gradlew :generator-common:producePatchAndVerify \
-PprevDb=$PWD/build/seforim.db.v1 \
-PnewDb=$PWD/build/seforim.db \
-Pout=$PWD/build/patch-v1-v2.db \
-PfromVersion=1 -PtoVersion=2 \
-PmaxDeltaUncompressedRatio=1000 \
--no-daemon --stacktrace

- name: Summarise patch
Expand Down
19 changes: 14 additions & 5 deletions .github/workflows/manual-generate-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2061,12 +2061,21 @@ jobs:
wait buildstate 3600
echo "=== Final patch artefacts ==="
ls -lh patches/ || true
# Skipped anchors are warnings, but a release with prior versions and
# NO delta at all means every anchor was skipped — that is a broken
# patch contract, not a degraded one, and must fail loudly.
# Skipped anchors are warnings. No delta AT ALL is only acceptable
# when the size guard is what removed them: a corpus-wide churn build
# legitimately ships full-only (seforim.db.zst + buildstate). Every
# anchor skipped for any other reason is still a broken patch
# contract and must fail loudly. SKIP_DIR is patch_fan_lib.sh's.
if ! compgen -G "patches/patch-*.db.zst" > /dev/null; then
echo "::error::patch fan produced no patch although prior releases exist (every anchor skipped)"
exit 1
OVERSIZED_SKIPS=$(find "$SKIP_DIR" -type f -name '*.oversized' | wc -l)
STRUCTURAL_SKIPS=$(find "$SKIP_DIR" -type f -name '*.structural' | wc -l)
if [ "$OVERSIZED_SKIPS" -gt 0 ]; then
echo "::warning::patch fan produced no patch: $OVERSIZED_SKIPS anchor(s) dropped by the delta size guard, $STRUCTURAL_SKIPS structurally unpatchable — publishing a full-only release (seforim.db.zst + buildstate)"
cat "$SKIP_DIR"/*.oversized
else
echo "::error::patch fan produced no patch although prior releases exist (every anchor skipped; $STRUCTURAL_SKIPS structurally unpatchable, none dropped by the size guard)"
exit 1
fi
fi

# Compress the full DB into a single .zst for the release. Done after the
Expand Down
26 changes: 25 additions & 1 deletion DELTA_UPDATE_WORKFLOW.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,38 @@ The allocator's natural keys are :
| `tocText` | display text | |
| `connection_type` | `name` | "commentary", "targum" |
| `book` | `(sourceName, canonicalHeTitle)` | survives renames via book_aliases |
| `line` | `(bookId, "REF:"+heRef)` for Sefaria, `(bookId, contentHash, occurrenceIdx)` for Otzaria | heRef is THE killer feature — survives Sefaria's prefix renumbering |
| `line` | `(bookId, sha1("CT:"+rawSegment), occurrenceIdx)` | content only — see the note below |
| `tocEntry` | `(bookId, ancestorPath@lineIndex)` | path is a `/`-joined sequence of tocText ids |
| `link` | `(srcLineId, tgtLineId, connectionTypeId)` | |

> If two builds share the same `build_state.db` seed and the corpus
> contents are unchanged, all ids match. If a row's content changes,
> only that row gets a new id; everything around it stays put.

> **`line` key, post-#1211.** Both Sefaria and Otzaria lines are keyed on
> content alone. Up to db v27 a Sefaria line with an `heRef` was keyed
> `"REF:"+heRef` instead; one reformatting pass that rewrote every heRef
> renumbered the whole corpus and produced a 3 GB delta, so the ref — an
> ordinary updatable column — is out of the key.
>
> `rawSegment` is the Sefaria segment **before** the generated prefixes the
> importer injects (`(א) `, daf labels). Hashing the rendered line would
> reintroduce the same failure at chapter scale: inserting one verse
> reprefixes every verse after it. `BookPayload.cleanShiftByLineIndex`
> records the injected prefix length, and the key strips it.
>
> Lines that repeat verbatim inside one book are separated by
> `occurrenceIdx`, a per-`(bookId, contentHash)` counter in document order.
>
> `LegacyLineKey` is the one-build migration shim: on a miss the allocator
> retries the pre-#1211 key (`"REF:"+heRef`, else `"CT:"+renderedContent`)
> for **every** line and re-files the id under the new key, so the snapshot
> it writes is fully migrated. The shim reads only the *seed* snapshot, and an
> id already issued to another line this build is never handed out again
> (`legacyLineKeyCollisions` in the build summary) — a line whose seed id was
> taken gets a fresh one rather than a duplicate. Delete the shim once no
> build_state in circulation predates the change.

### 1.2 The full producer pipeline

```mermaid
Expand Down
35 changes: 19 additions & 16 deletions LINKER_DELTA_PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@
ל-שורה **בזמן בניית ה-DB** דרך ה-resolver הקיים של הגנרטור. כך:

- כשספריא משנה ספר-יעד, ה-ref פשוט נפתר לשורה החדשה — **בלי לגעת בקישור**.
- הגנרטור כבר ממפתח `lineId` של שורות ספריא לפי **`REF:$heRef`**
(`IdAllocatorBindings.lineNaturalKeyHash`), כך ש-**כל עוד ה-heRef של הפסוק לא
השתנה, ה-`lineId` יציב בין builds → ה-`linkId` יציב → הקישור שורד עדכון-תוכן
של ספריא ללא שום churn ב-delta.**
- הגנרטור ממפתח `lineId` לפי **תוכן השורה הגולמי** בלבד
(`IdAllocatorBindings.lineNaturalKeyHash` = `sha1("CT:"+rawSegment)`, ללא
ה-heRef וללא הקידומת המיוצרת), כך ש-**כל עוד טקסט הפסוק לא השתנה, ה-`lineId`
יציב בין builds → ה-`linkId` יציב → הקישור שורד עדכון-מבנה או שינוי-ref של
ספריא ללא שום churn ב-delta.**
> עד db v27 המפתח היה `REF:$heRef`, ומעבר-עיצוב שכתב מחדש כל heRef מספרר
> מחדש את כל הקורפוס (issue #1211). ה-ref הוא עמודה מתעדכנת, לא מפתח.

זו התשובה ל"התאמת קישורים לספרים שהשתנו": **ההתאמה אוטומטית דרך פתרון-מחדש**,
לא הגירה. הצעד היקר (NER) מתבצע רק על ספרי-**מקור** שהשתנו.
Expand Down Expand Up @@ -111,7 +114,7 @@
| המרה + מיזוג (מחליף רק ערכי "linker") | `otzaria-library/linker/to_otzaria_links.py` | ⚠️ קיים, **מושבת** |
| פתרון ref→שורה | `SefariaImportRefs.resolveRefs` (+ `refsByCanonical/refsByBase`) | ✅ קיים, בדוק |
| ‏id יציב + patch DB-ל-DB | `InMemoryIdAllocator.linkId`, `PatchDbProducer`, `PatchTables` | ✅ קיים |
| ‏line-id יציב לפי ref | `lineNaturalKeyHash` = `REF:$heRef` | ✅ קיים (הלב של היציבות) |
| ‏line-id יציב לפי תוכן | `lineNaturalKeyHash` = `sha1("CT:"+rawSegment)` | ✅ קיים (הלב של היציבות) |
| מעקב ספרי-מקור שהשתנו בבנייה | `SourceHashComputer` + `TouchedBookDetector` + `BookRenameDetector` | ✅ קיים |

**המסקנה:** אין לבנות תשתית delta מאפס — יש **להחיות ולחבר**.
Expand Down Expand Up @@ -173,23 +176,23 @@ lineIndex`) **וגם את `lineId` המוטבע לכל רשומה** (זהות-ה
| שינוי | השפעה | טיפול | עלות |
|---|---|---|---|
| **ספר-מקור (otzaria) שינה תוכן** | מיקומי ציטוט זזים | הרצת לינקר מחדש על הספר בלבד | דקות (NER על ספר) |
| **ספר-יעד (ספריא) שינה תוכן, heRef נשמר** | השורה זזה | פתרון-מחדש בבנייה; `lineId` יציב (REF-keyed) | **אפס** — קישור שורד, בלי churn |
| **ספר-יעד שינה מבנה (heRef השתנה/נמחק)** | ref לא נפתר | קישור מתעדכן/נופל + מדווח (כמו קישורי ספריא) | אפס לינקר |
| **ספר-יעד (ספריא) שינה מבנה/heRef, טקסט הפסוק נשמר** | השורה זזה | פתרון-מחדש בבנייה; `lineId` יציב (content-keyed) | **אפס** — קישור שורד, בלי churn |
| **ספר-יעד שינה את טקסט הפסוק עצמו** | `lineId` מתחלף | הקישור נמחק+נכתב מחדש ב-patch | churn מקומי (הפסוק אכן השתנה) |
| **ספר-יעד שונה-שם/הועבר** | ref-מחרוזת מיושן | rewrite לפי `changelog_diff.json` | זול (מחרוזות) |
| **ספר-מקור שונה-שם/נמחק** | קובץ-ארטיפקט מיושן | move/delete (לוגיקת `linker_on_commit.py`) | זול |

> **הערך:** התרחיש התכוף והמסוכן ביותר — עדכון-תוכן של ספר-יעד ספריא —
> **הערך:** התרחיש התכוף והמסוכן ביותר — עדכון-מבנה של ספר-יעד ספריא —
> עולה **אפס** בלינקר ואינו שובר קישורים, בזכות שילוב ref-artifact + line-id
> ממופתח-heRef. זה מה שהופך את זה מ"שעות ריצה שמתפוצצות" ל"בנייה אינקרמנטלית".
> ממופתח-תוכן. זה מה שהופך את זה מ"שעות ריצה שמתפוצצות" ל"בנייה אינקרמנטלית".

### 2ה. מגבלה כנה — צד-המקור של ספרי otzaria
### 2ה. מגבלה כנה — עריכת טקסט מחליפה `lineId`

שורות של ספרי otzaria ממופתחות ל-`lineId` לפי **hash-תוכן** (`stableLineId` →
`normalisedContentHash`, בלי קידומת ref). לכן עריכה של שורת-מקור otzaria מחליפה
את `srcLineId` → `linkId` משתנה → הקישור נמחק+נכתב מחדש ב-patch (churn) גם אם
הציטוט טקסטואלית זהה. זה **מקובל** (הספר השתנה) אבל מייצר delta גדול מהמינימלי.
שיפור עתידי אפשרי: מפתח-טבעי יציב יותר לשורות otzaria. **לא חוסם** — היציבות
החשובה (צד-יעד/עדכוני-ספריא, המקרה התכוף) פתורה במלואה.
כל השורות, בשני הצדדים, ממופתחות ל-`lineId` לפי **hash-תוכן**. לכן עריכה של
שורת-מקור מחליפה את `srcLineId` → `linkId` משתנה → הקישור נמחק+נכתב מחדש
ב-patch (churn) גם אם הציטוט טקסטואלית זהה. זה **מקובל** (השורה אכן השתנתה)
אבל מייצר delta גדול מהמינימלי. שיפור עתידי אפשרי: התאמת-שורות מנורמלת
(`LineMatcher`) שתשמר id על עריכה קלה. **לא חוסם** — היציבות החשובה (שינויי
מבנה ו-ref בספריא, המקרה התכוף) פתורה במלואה.

---

Expand Down
2 changes: 1 addition & 1 deletion generator/common/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ tasks.register<JavaExec>("producePatchAndVerify") {
listOf(
"releaseMeta", "fullBundleUrl", "fullBundleSha", "fullBundleSize",
"manifestBaseUrl", "fromSchemaVersion", "toSchemaVersion",
"catalogPb", "zstdLevel",
"catalogPb", "zstdLevel", "maxDeltaUncompressedRatio",
).forEach { key ->
project.findProperty(key)?.let { systemProperty(key, it as String) }
}
Expand Down
Loading
Loading