diff --git a/README.md b/README.md index faaf154..a5e9a70 100644 --- a/README.md +++ b/README.md @@ -37,8 +37,10 @@ lock pins the edition, so a healed book *stays* healed (set-once, no oscillation the metadata provider for candidates and asks an LLM to pick the *same work* — but only from the retrieved set, validated against it (the model can never invent an id), and only auto-applies above a confidence threshold. Below it, or on doubt: left flagged. -- **Series audit** — compares each book's series number against the provider's - authoritative position and heals genuine mismatches (read-only by default). +- **Series audit** — compares each book's series number *and* grouping against the provider's + authoritative series, and heals genuine mismatches: a wrong/missing number, an ungrouped book (no + series name), or a variant series name where the title still matches the provider. A true mis-seed + (name *and* title disagree) is left for the resolver. Read-only by default; runs nightly. - **Dedup** — collapses duplicate records, moving the loser's file onto the keeper as an *alternative format* (nothing deleted from disk) and removing the empty record. - **Oversight** — a weekly changelog review that flags drift (a book healed more than @@ -100,7 +102,7 @@ python -m colophon.cli backfill --apply # heal broken ISBNs python -m colophon.cli enrich --apply # seed bare no-id imports; remember the unresolvable python -m colophon.cli resolve --apply # LLM-resolve mis-identified books (>= 0.9 conf) python -m colophon.cli resolve --force # re-query the cached-unresolvable mis-seeds -python -m colophon.cli series-audit # series-number report (read-only) +python -m colophon.cli series-audit # series numbering + grouping report (read-only) python -m colophon.cli maintain --apply --email # backfill + resolve in one run + summary email python -m colophon.cli oversight --days 7 # weekly changelog review + verdict python -m colophon.cli log # the change history diff --git a/colophon/cli.py b/colophon/cli.py index 1cd5e44..7b40e83 100644 --- a/colophon/cli.py +++ b/colophon/cli.py @@ -298,9 +298,10 @@ def main(argv=None): rs.add_argument("--min-conf", type=float, default=0.9) rs.add_argument("--force", action="store_true", help="re-query books on the cached-unresolvable skip-list") rs.add_argument("--clear-skips", action="store_true", help="forget all cached-unresolvable entries, then exit") - sn = sub.add_parser("series-audit", help="Phase 3 — series-numbering audit (read-only unless --apply)") + sn = sub.add_parser("series-audit", help="Phase 3 — series numbering + grouping audit (read-only unless --apply)") sn.add_argument("--limit", type=int, default=None) - sn.add_argument("--apply", action="store_true", help="heal clean number mismatches (reuses the heal path)") + sn.add_argument("--apply", action="store_true", + help="heal clean number-mismatch / number-missing / series-name-missing (reuses the heal path)") ov = sub.add_parser("oversight", help="Phase 4b — weekly changelog oversight + verdict (emails only if flagged)") ov.add_argument("--days", type=int, default=7) ov.add_argument("--email", action="store_true", help="email the digest only when flagged (DRIFT/REVIEW)") diff --git a/colophon/maintain.py b/colophon/maintain.py index c10ef86..102e744 100644 --- a/colophon/maintain.py +++ b/colophon/maintain.py @@ -12,14 +12,14 @@ """ import time -from . import backfill, grimmory +from . import backfill, grimmory, series_audit from .heal import assert_preconditions from .resolver import run_resolve def run_maintain(g, store, limit=20, min_conf=0.9, apply=False, force=False): res = {"ts": time.strftime("%Y-%m-%d %H:%M"), "apply": apply, "ok": True, - "aborted": False, "backfill": None, "resolve": None, "errors": []} + "aborted": False, "backfill": None, "resolve": None, "series": None, "errors": []} if apply: try: assert_preconditions(g) @@ -42,6 +42,15 @@ def run_maintain(g, store, limit=20, min_conf=0.9, apply=False, force=False): except Exception as e: # noqa: BLE001 res["ok"] = False res["errors"].append(f"resolve: {str(e)[:200]}") + # Series: numbering + grouping. Auto-heals clean number-mismatch/number-missing + # and the ungrouped series-name-missing cases; series-mismatch stays for resolve. + try: + res["series"] = series_audit.run(apply=apply, g=g, store=store) + if res["series"]["errors"]: + res["ok"] = False + except Exception as e: # noqa: BLE001 + res["ok"] = False + res["errors"].append(f"series: {str(e)[:200]}") # Informational only — surfacing the enrich stuck-list must never fail the run. res["stuck"] = [] try: @@ -76,7 +85,8 @@ def verdict(res): def _healed_count(res): bf = (res["backfill"] or {}).get("healed", 0) rs = sum(1 for p in (res["resolve"] or {}).get("proposals", []) if p.get("applied")) - return bf + rs + sa = (res.get("series") or {}).get("healed", 0) + return bf + rs + sa def subject(res): @@ -132,6 +142,21 @@ def render_summary(res): else: L.append("Resolve: did not run") + sa = res.get("series") + if sa: + cats = sa["categories"] + smis = len(cats.get("series-mismatch", [])) + L.append(f"Series (numbering + grouping): {sa['total']} scanned · {sa['healed']} healed · " + f"{smis} series-mismatch → resolver · {sa['errors']} errors" + + (" ABORTED (circuit-breaker)" if sa.get("errors", 0) >= series_audit.ABORT_ERRORS else "")) + for k in ("number-mismatch", "number-missing", "series-name-missing", "series-name-variant"): + for rec in cats.get(k, []): + if rec.get("applied"): + b = rec["book"] + L.append(f" + book {b['book_id']} {b['title']!r} — {rec['reason']}") + else: + L.append("Series: did not run") + stuck = res.get("stuck") or [] if stuck: L.append("") diff --git a/colophon/series_audit.py b/colophon/series_audit.py index a0563d4..fa644a6 100644 --- a/colophon/series_audit.py +++ b/colophon/series_audit.py @@ -7,19 +7,36 @@ and grimmory repopulates series_number from Hardcover's position (verified live on book 590 "Zero Hour": German-edition ISBN → number 2; heal → canonical ISBN → number 5). -Deliberately NOT wired into the daily sweep (numbering is the least-tested surface and -overlaps dedup + mis-seed resolution). Read-only `series-audit`; `--apply` heals only -the clean number mismatches, gated, dry-run by default. +Runs as a guarded phase of the nightly sweep (`maintain`) and standalone via +`series-audit` (read-only unless `--apply`). `--apply` heals the clean cases — +number-mismatch / number-missing and the grouping repairs series-name-missing / +series-name-variant — gated, dry-run by default. Only a *true* mis-seed +(series-mismatch: the name AND the title disagree with the hcid) stays deferred +to the resolver, which validates the identity against candidates before locking. Categories: - number-mismatch : series matches, grimmory number != Hardcover position → FIX (heal) - number-missing : series matches, grimmory number null, Hardcover has one → FIX (heal) - series-mismatch : grimmory series != hcid's series → likely a mis-seed → resolver - dup-overlap : hcid shared with another owned book → plan 21 - no-position : Hardcover has no position for this id → leave - no-hcid : in a series but unidentified → leave - number-ok : grimmory number == Hardcover position + number-mismatch : series matches, grimmory number != Hardcover position → FIX (heal) + number-missing : series matches, grimmory number null, Hardcover has one → FIX (heal) + series-name-missing : grimmory has no series_name, Hardcover puts it in one → FIX (heal) + series-name-variant : name differs but the book TITLE matches the hcid → FIX (heal) + series-mismatch : name AND title differ → the hcid itself is suspect → resolver + dup-overlap : hcid shared with another owned book → plan 21 + no-position : Hardcover has no position for this id → leave + no-series : no series in grimmory or Hardcover (standalone) → leave + no-hcid : in a series but unidentified → leave + number-ok : grimmory number == Hardcover position + +The `series-name-missing` path (Symptom 1 — owned books that fall out of their +series because grimmory never derived a `series_name`) reuses the exact heal +recipe: set the canonical ISBN + hcid, lock, refresh, and grimmory repopulates +BOTH series_name and series_number. It heals only books on a *non-canonical* +13-char edition (current ISBN != Hardcover's canonical) — books already on the +canonical edition with a null name are a grimmory-side derivation gap, not ours +to churn on. Broken-ISBN books are left to the backfill phase. A *disagreeing* +name (series-mismatch) is still deferred to the resolver: the disagreement is +itself evidence the hcid may be wrong, so it needs adjudication before a lock. """ +import re import time from collections import defaultdict @@ -27,26 +44,56 @@ from .heal import assert_preconditions, heal_book ABORT_ERRORS = 3 +UNGROUPED_LIMIT = 60 # per-run cap on the ungrouped (null series_name) survey _SQL = ( "SELECT bm.book_id, IFNULL(bm.title,''), IFNULL(bm.hardcover_book_id,''), " "IFNULL(bm.series_name,''), IFNULL(bm.series_number,''), IFNULL(bm.isbn_13,''), " - "IFNULL(bm.series_number_locked,0) " + "IFNULL(bm.series_number_locked,0), IFNULL(bm.series_name_locked,0) " "FROM book_metadata bm JOIN book b ON b.id=bm.book_id " "WHERE (b.deleted IS NULL OR b.deleted=0) " "AND bm.series_name IS NOT NULL AND bm.series_name<>'';" ) +_UNGROUPED_SQL = ( + "SELECT bm.book_id, IFNULL(bm.title,''), IFNULL(bm.hardcover_book_id,''), " + "IFNULL(bm.isbn_13,''), IFNULL(bm.series_name_locked,0) " + "FROM book_metadata bm JOIN book b ON b.id=bm.book_id " + "WHERE (b.deleted IS NULL OR b.deleted=0) " + "AND bm.hardcover_book_id IS NOT NULL AND bm.hardcover_book_id<>'' " + "AND (bm.series_name IS NULL OR bm.series_name='') " + "AND bm.isbn_13 IS NOT NULL AND LENGTH(REPLACE(bm.isbn_13,'-',''))=13 " + "ORDER BY bm.book_id LIMIT {limit};" +) + + def _series_books(): out, rows = grimmory._db(_SQL), [] for line in out.splitlines(): c = line.split("\t") - if len(c) < 7: + if len(c) < 8: continue rows.append({"book_id": int(c[0]), "title": c[1], "hcid": c[2].strip(), "series_name": c[3], "series_number": c[4].strip(), - "isbn": c[5].strip(), "num_locked": c[6] == "1"}) + "isbn": c[5].strip(), "num_locked": c[6] == "1", "name_locked": c[7] == "1"}) + return rows + + +def _ungrouped_candidates(limit): + """hcid'd books with a clean 13-char ISBN but no series_name — possible + ungrouped series members (Symptom 1). Bounded: the cap keeps the nightly + Hardcover lookups cheap; healed books gain a name and drop out next run.""" + if not limit: + return [] + out, rows = grimmory._db(_UNGROUPED_SQL.format(limit=int(limit))), [] + for line in out.splitlines(): + c = line.split("\t") + if len(c) < 5: + continue + rows.append({"book_id": int(c[0]), "title": c[1], "hcid": c[2].strip(), + "series_name": "", "series_number": "", "isbn": c[3].strip(), + "num_locked": False, "name_locked": c[4] == "1"}) return rows @@ -57,6 +104,10 @@ def _as_float(s): return None +def _isbn_digits(s): + return re.sub(r"\D", "", s or "") + + def audit_one(b, hcid_counts): """Return (category, reason, fix|None). fix = (isbn, hcid) to heal, when applicable.""" hcid = b["hcid"] @@ -70,10 +121,40 @@ def audit_one(b, hcid_counts): if not cand: return "series-mismatch", f"hcid {hcid} not found in Hardcover", None hc_series, hc_pos, hc_isbn = cand.get("series"), cand.get("position"), cand.get("isbn") + if not b["series_name"]: + if not hc_series: + return "no-series", "standalone — no series in grimmory or Hardcover", None + if b.get("name_locked"): + return "series-name-missing", f"ungrouped from {hc_series!r} (series_name LOCKED — manual)", None + if not hc_isbn: + return "series-name-missing", f"ungrouped from {hc_series!r} — no canonical ISBN to heal", None + if _isbn_digits(b["isbn"]) == _isbn_digits(hc_isbn): + return ("series-name-missing", + f"ungrouped from {hc_series!r}; already on canonical ISBN — grimmory derived no series (leave)", + None) + pos = "" if hc_pos is None else f" #{float(hc_pos):g}" + return "series-name-missing", f"ungrouped → series {hc_series!r}{pos}", (hc_isbn, hcid) if hc_series and not matcher._title_match(b["series_name"], hc_series): - return ("series-mismatch", - f"grimmory series {b['series_name']!r} != hcid series {hc_series!r} → mis-seed (resolver)", - None) + # Series name disagrees. If the book TITLE corroborates the hcid, it's a + # stale/variant name — heal to canonical so grimmory re-derives the right + # one (same heal as number-mismatch). If the title ALSO disagrees, the + # hcid itself is suspect (a real mis-seed): leave it for the resolver, + # which validates the identity against candidates before locking. + if not (cand.get("title") and matcher._title_match(b["title"], cand["title"])): + return ("series-mismatch", + f"grimmory series {b['series_name']!r} != hcid series {hc_series!r} → mis-seed (resolver)", + None) + if b["name_locked"]: + return "series-name-variant", f"variant name {b['series_name']!r} → {hc_series!r} (series_name LOCKED — manual)", None + if not hc_isbn: + return "series-name-variant", f"variant name {b['series_name']!r} → {hc_series!r} — no canonical ISBN to heal", None + if _isbn_digits(b["isbn"]) == _isbn_digits(hc_isbn): + return ("series-name-variant", + f"variant name {b['series_name']!r} → {hc_series!r}; already on canonical ISBN (leave)", + None) + return ("series-name-variant", + f"grimmory series {b['series_name']!r} → {hc_series!r} (title matches hcid)", + (hc_isbn, hcid)) if hc_pos is None: return "no-position", "Hardcover has no series position for this id", None gn = _as_float(b["series_number"]) @@ -95,10 +176,13 @@ def audit_one(b, hcid_counts): return cat, reason, fix -def run(limit=None, apply=False, g=None, store=None): +def run(limit=None, apply=False, g=None, store=None, ungrouped_limit=None): books = _series_books() if limit: books = books[:limit] + # Default cap walks the ungrouped set nightly; a targeted --limit mirrors it. + ug_cap = ungrouped_limit if ungrouped_limit is not None else (limit or UNGROUPED_LIMIT) + books += _ungrouped_candidates(ug_cap) hcid_counts = defaultdict(int) for b in books: if b["hcid"]: @@ -111,7 +195,8 @@ def run(limit=None, apply=False, g=None, store=None): for b in sorted(books, key=lambda x: (x["series_name"], _as_float(x["series_number"]) or 0)): cat, reason, fix = audit_one(b, hcid_counts) rec = {"book": b, "reason": reason, "fix": fix, "applied": False} - if apply and fix and cat in ("number-mismatch", "number-missing"): + if apply and fix and cat in ("number-mismatch", "number-missing", + "series-name-missing", "series-name-variant"): try: heal_book(g, store, run_id, b["book_id"], fix[0], fix[1], None, dry_run=False) rec["applied"] = True @@ -128,13 +213,16 @@ def run(limit=None, apply=False, g=None, store=None): "healed": healed, "errors": errors, "apply": apply} -_ORDER = ["number-mismatch", "number-missing", "series-mismatch", "dup-overlap", - "no-position", "no-hcid", "error", "number-ok"] +_ORDER = ["number-mismatch", "number-missing", "series-name-missing", "series-name-variant", + "series-mismatch", "dup-overlap", "no-position", "no-series", "no-hcid", "error", "number-ok"] _LABEL = {"number-mismatch": "Wrong number (heal-fixable)", "number-missing": "Missing number (heal-fixable)", + "series-name-missing": "Ungrouped — missing series name (heal-fixable)", + "series-name-variant": "Variant series name (heal-fixable)", "series-mismatch": "Series mismatch → mis-seed (resolver)", "dup-overlap": "Duplicate hcid → dedup (plan 21)", "no-position": "Hardcover has no position (leave)", + "no-series": "Standalone — not in any series (leave)", "no-hcid": "Unidentified in a series (leave)", "error": "Provider lookup error", "number-ok": "Correct"} @@ -150,7 +238,7 @@ def render(res): if cats.get(k): L.append(f"- **{len(cats[k])}** {_LABEL[k]}") for k in _ORDER: - if k == "number-ok" or not cats.get(k): + if k in ("number-ok", "no-series") or not cats.get(k): continue L.append(f"\n## {_LABEL[k]} ({len(cats[k])})\n") for rec in sorted(cats[k], key=lambda r: (r["book"]["series_name"], r["book"]["book_id"])): diff --git a/tests/test_series_audit.py b/tests/test_series_audit.py new file mode 100644 index 0000000..c87cc51 --- /dev/null +++ b/tests/test_series_audit.py @@ -0,0 +1,174 @@ +"""series_audit categorization + the maintain series phase. No network, no writes. +Run: python -m unittest -v + +audit_one is pure once the Hardcover lookup is stubbed, so most coverage targets it +directly. The grouping (series-name-missing) path is the new surface (Symptom 1). +""" +import unittest + +from colophon import maintain, series_audit + + +def _stub_lookup(mapping): + """Replace series_audit's Hardcover lookup with a dict keyed by hcid.""" + series_audit.audit._book_by_id = lambda hcid: mapping.get(str(hcid)) + + +def _row(**kw): + d = {"book_id": 1, "title": "T", "hcid": "100", "series_name": "", "series_number": "", + "isbn": "", "num_locked": False, "name_locked": False} + d.update(kw) + return d + + +class AuditOne(unittest.TestCase): + def setUp(self): + self._orig = series_audit.audit._book_by_id + + def tearDown(self): + series_audit.audit._book_by_id = self._orig + + def test_ungrouped_heals_to_canonical(self): + # null series_name + non-canonical 13-char ISBN + Hardcover series → heal. + _stub_lookup({"100": {"series": "Foundation", "position": 2, + "isbn": "9780553293357"}}) + b = _row(series_name="", isbn="9788424117788") # foreign edition + cat, reason, fix = series_audit.audit_one(b, {}) + self.assertEqual(cat, "series-name-missing") + self.assertEqual(fix, ("9780553293357", "100")) + self.assertIn("Foundation", reason) + + def test_ungrouped_standalone_is_left(self): + _stub_lookup({"100": {"series": None, "position": None, "isbn": "9780553293357"}}) + cat, _, fix = series_audit.audit_one(_row(isbn="9780553293357"), {}) + self.assertEqual(cat, "no-series") + self.assertIsNone(fix) + + def test_ungrouped_already_canonical_not_reheal(self): + # Same ISBN as Hardcover's canonical → re-heal would not change identity; + # leave it (a grimmory derivation gap, not ours to churn on). + _stub_lookup({"100": {"series": "Dune", "position": 1, "isbn": "9780441013593"}}) + cat, reason, fix = series_audit.audit_one(_row(isbn="978-0-441-01359-3"), {}) + self.assertEqual(cat, "series-name-missing") + self.assertIsNone(fix) + self.assertIn("already on canonical", reason) + + def test_ungrouped_name_locked_not_healed(self): + _stub_lookup({"100": {"series": "Dune", "position": 1, "isbn": "9780441013593"}}) + cat, reason, fix = series_audit.audit_one(_row(isbn="9788424117788", name_locked=True), {}) + self.assertEqual(cat, "series-name-missing") + self.assertIsNone(fix) + self.assertIn("LOCKED", reason) + + def test_no_hcid_left(self): + cat, _, fix = series_audit.audit_one(_row(hcid="", series_name="Dune"), {}) + self.assertEqual(cat, "no-hcid") + self.assertIsNone(fix) + + def test_number_mismatch_still_heals(self): + _stub_lookup({"100": {"series": "Dune", "position": 5, "isbn": "9780441013593"}}) + b = _row(series_name="Dune", series_number="2", isbn="9788424117788") + cat, _, fix = series_audit.audit_one(b, {}) + self.assertEqual(cat, "number-mismatch") + self.assertEqual(fix, ("9780441013593", "100")) + + def test_number_ok(self): + _stub_lookup({"100": {"series": "Dune", "position": 5, "isbn": "9780441013593"}}) + b = _row(series_name="Dune", series_number="5") + cat, _, fix = series_audit.audit_one(b, {}) + self.assertEqual(cat, "number-ok") + self.assertIsNone(fix) + + def test_series_name_variant_heals_when_title_matches(self): + # Name differs but the book TITLE corroborates the hcid → it's a stale + # variant name; heal to canonical so grimmory re-derives the right one. + _stub_lookup({"100": {"title": "The Way of Kings", "series": "The Stormlight Archive", + "position": 1, "isbn": "9780765326355"}}) + b = _row(title="The Way of Kings", series_name="Cosmere Saga", + series_number="1", isbn="9788401021234") # foreign edition + cat, _, fix = series_audit.audit_one(b, {}) + self.assertEqual(cat, "series-name-variant") + self.assertEqual(fix, ("9780765326355", "100")) + + def test_series_name_variant_already_canonical_left(self): + _stub_lookup({"100": {"title": "The Way of Kings", "series": "The Stormlight Archive", + "position": 1, "isbn": "9780765326355"}}) + b = _row(title="The Way of Kings", series_name="Cosmere Saga", isbn="9780765326355") + cat, _, fix = series_audit.audit_one(b, {}) + self.assertEqual(cat, "series-name-variant") + self.assertIsNone(fix) + + def test_series_mismatch_deferred_when_title_also_differs(self): + # Name AND title disagree → the hcid itself is suspect (a real mis-seed): + # leave it for the resolver, do NOT lock a wrong identity. + _stub_lookup({"100": {"title": "Dune", "series": "Dune", "position": 1, + "isbn": "9780441013593"}}) + b = _row(title="The Hobbit", series_name="Middle-earth", series_number="1") + cat, _, fix = series_audit.audit_one(b, {}) + self.assertEqual(cat, "series-mismatch") + self.assertIsNone(fix) + + +class RunSurvey(unittest.TestCase): + """run() must fold the ungrouped survey in and categorize it (apply=False).""" + + def setUp(self): + self._db, self._lookup = series_audit.grimmory._db, series_audit.audit._book_by_id + + def fake_db(sql): + if "series_name IS NULL OR" in sql: # ungrouped survey + return "9\tThe Ungrouped One\t200\t9788424117788\t0" + return "" # no books already carrying a series_name + + series_audit.grimmory._db = fake_db + _stub_lookup({"200": {"series": "Foundation", "position": 3, "isbn": "9780553293357"}}) + + def tearDown(self): + series_audit.grimmory._db, series_audit.audit._book_by_id = self._db, self._lookup + + def test_ungrouped_book_is_categorized(self): + res = series_audit.run(apply=False) + self.assertEqual(res["total"], 1) + recs = res["categories"]["series-name-missing"] + self.assertEqual(len(recs), 1) + self.assertEqual(recs[0]["fix"], ("9780553293357", "200")) + + +class MaintainSeriesPhase(unittest.TestCase): + def setUp(self): + self._bf, self._rs, self._sa, self._stuck = ( + maintain.backfill.run, maintain.run_resolve, maintain.series_audit.run, + maintain._gather_stuck) + maintain.backfill.run = lambda *a, **k: {"errors": 0, "aborted": False, "proposals": [], "healed": 0} + maintain.run_resolve = lambda *a, **k: {"proposals": []} + maintain._gather_stuck = lambda store: [] + + def tearDown(self): + (maintain.backfill.run, maintain.run_resolve, maintain.series_audit.run, + maintain._gather_stuck) = self._bf, self._rs, self._sa, self._stuck + + def test_series_phase_runs(self): + maintain.series_audit.run = lambda **k: { + "total": 3, "healed": 1, "errors": 0, "run_id": None, "apply": False, + "categories": {"series-name-missing": [ + {"applied": True, "book": {"book_id": 9, "title": "X"}, "reason": "ungrouped → series 'Y'"}]}} + res = maintain.run_maintain(None, None, apply=False) + self.assertTrue(res["ok"]) + self.assertEqual(res["series"]["healed"], 1) + self.assertIn("Series (numbering + grouping)", maintain.render_summary(res)) + + def test_series_failure_isolated(self): + def _boom(**k): + raise RuntimeError("hardcover down") + + maintain.series_audit.run = _boom + res = maintain.run_maintain(None, None, apply=False) + self.assertFalse(res["ok"]) + self.assertTrue(any(e.startswith("series:") for e in res["errors"])) + # The earlier phases still ran — one failure must not skip the rest. + self.assertIsNotNone(res["backfill"]) + self.assertIsNotNone(res["resolve"]) + + +if __name__ == "__main__": + unittest.main()