diff --git a/CLAUDE.md b/CLAUDE.md index c64c130..667a5c0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -47,11 +47,11 @@ Full design lives in `bgg-shelf-pipeline-spec.md` (read it before changing pipel - Base game vs. expansion vs. new edition is the top failure mode — bias matching toward `ambiguous` over auto-match ("Wingspan Europe" must not match base Wingspan). - Editions/versions matter: Eric owns multiple editions of some games — each is a separate collection entry (keyed by `collid` on BGG). Never guess a version: no legible cues → `version_unknown` and a version-less collection entry. - Normalize titles (casefold, strip punctuation/articles, special chars like é/&/:) identically on both sides of a match; dedupe across photos but keep `source_photos` provenance. -- **Human curation is durable**: `data/title_splits.json` (photo-scoped split-into-copies decisions, honored by extract's dedupe AND resolve's dedupe) and `data/title_edits.json` (corrected reads/cues, applied before dedupe on every titles.json rebuild) persist forever. Row-level decisions persist via the `dedupe_veto` column — edits never drop veto'd rows (a rename retitles them in place). +- **Human curation is durable**: `data/title_splits.json` (photo-scoped split-into-copies decisions, honored by extract's dedupe AND resolve's dedupe), `data/title_edits.json` (corrected reads/cues, applied before dedupe on every titles.json rebuild), and `data/title_removals.json` (lines removed from the catalog — filtered out of every rebuild; delete the record to undo) persist forever. Row-level decisions persist via the `dedupe_veto` column — edits never drop veto'd rows (a rename retitles them in place); removal drops them (explicitly discarding the line). - **RPGs are local-only citizens**: when the board-game search runs dry, resolve falls back to `type=rpgitem` (same geekdo API/token). Matched rpgitems enrich into the library but diff routes them to `local_only` — they must never reach `to_add.csv`/upload (their collection lives on RPGGeek, out of scope). - Detailed BGG API behavior (202 queueing, collection-endpoint quirks, endpoints): use the `bgg-api` skill. **If the spec's BGG behavior changes, update the `bgg-api` skill to match** — they must not drift. ## Git - Remote is self-hosted Gitea 1.26 (`git.kestrelsnest.social/eric/bggpipe`), **not GitHub** — `gh` CLI does not work here. -- Commit `data/matches.csv`, `data/to_add.csv`, `data/to_update.csv`, `data/upload_log.csv`, `data/titles.json`, `data/unidentified.json`, `data/unidentified_dismissed.json`, `data/title_splits.json`, `data/title_edits.json`, `data/games.json`, `data/STUB_DATA.marker` (while it applies), and the collection snapshot XMLs. Never commit `data/bgg_cache/`, `data/extract_raw/`, `photos/`, Playwright storage state, or `.env`. +- Commit `data/matches.csv`, `data/to_add.csv`, `data/to_update.csv`, `data/upload_log.csv`, `data/titles.json`, `data/unidentified.json`, `data/unidentified_dismissed.json`, `data/title_splits.json`, `data/title_edits.json`, `data/title_removals.json`, `data/games.json`, `data/STUB_DATA.marker` (while it applies), and the collection snapshot XMLs. Never commit `data/bgg_cache/`, `data/extract_raw/`, `photos/`, Playwright storage state, or `.env`. diff --git a/data/title_edits.json b/data/title_edits.json new file mode 100644 index 0000000..2e4d3cb --- /dev/null +++ b/data/title_edits.json @@ -0,0 +1,6 @@ +[ + { + "match": "Hangermuger 4 800", + "title_raw": "Huggermugger" + } +] diff --git a/data/titles.json b/data/titles.json index aadd394..6f5c15c 100644 --- a/data/titles.json +++ b/data/titles.json @@ -783,7 +783,7 @@ "title_normalized": "dixit" }, { - "title_raw": "Hangermuger 4 800", + "title_raw": "Huggermugger", "confidence": "low", "publisher_hint": "", "edition_hint": "", @@ -793,7 +793,7 @@ "source_photos": [ "IMG_4514.jpeg" ], - "title_normalized": "hangermuger 4 800" + "title_normalized": "huggermugger" }, { "title_raw": "The Ain't It Cool Trivia Game", diff --git a/src/bggpipe/config.py b/src/bggpipe/config.py index d834cdb..a158a29 100644 --- a/src/bggpipe/config.py +++ b/src/bggpipe/config.py @@ -66,6 +66,12 @@ class Config: def games_path(self) -> Path: return self.data_dir / "games.json" + @property + def title_removals_path(self) -> Path: + # titles the human removed from the catalog (not a game, misread) — + # filtered out of every titles.json rebuild + return self.data_dir / "title_removals.json" + @property def title_edits_path(self) -> Path: # human corrections to extracted reads (misspellings, known cues) — diff --git a/src/bggpipe/extract.py b/src/bggpipe/extract.py index 8c078f6..bf7023a 100644 --- a/src/bggpipe/extract.py +++ b/src/bggpipe/extract.py @@ -313,11 +313,10 @@ def apply_title_edits(entries: list[dict], edits: list[dict]) -> list[dict]: return entries -def load_title_splits(path: Path) -> list[dict]: - """The human's split-into-copies decisions — durable: they must survive - extract rebuilds and resolve --force. Each stored record is - {"title": ..., "photos": [...]} scoping the split to the sightings that - were on the split line (a bare string is legacy: unscoped). Returns +def _scoped_records(path: Path) -> list[dict]: + """Parse a store of photo-scoped title decisions. Each stored record is + {"title": ..., "photos": [...]} scoping the decision to the sightings + that were on the line (a bare string is legacy: unscoped). Returns [{"norm": , "photos": set | None}].""" records = [] for item in _load_store(path): @@ -334,6 +333,20 @@ def load_title_splits(path: Path) -> list[dict]: return records +def load_title_splits(path: Path) -> list[dict]: + """The human's split-into-copies decisions — durable: they must survive + extract rebuilds and resolve --force.""" + return _scoped_records(path) + + +def load_title_removals(path: Path) -> list[dict]: + """Titles the human removed from the catalog (not a game, a misread of + box art, out of scope) — their sightings are filtered out of every + rebuild, so re-extraction cannot resurrect them. Undo by deleting the + record from the store file.""" + return _scoped_records(path) + + def is_split(norm: str, photos, splits: list[dict]) -> bool: """Does a split decision cover this (normalized title, photo set)? Photo-scoped records only bind sightings from the photos that were on @@ -345,9 +358,9 @@ def is_split(norm: str, photos, splits: list[dict]) -> bool: ) -def record_title_split(path: Path, title: str, photos: list[str] | None = None) -> None: - if is_split(normalize_title(title), photos or [], load_title_splits(path)): - return +def _record_scoped(path: Path, title: str, photos: list[str] | None) -> None: + if is_split(normalize_title(title), photos or [], _scoped_records(path)): + return # an existing record already covers this line existing = _load_store(path) record = {"title": title, "photos": sorted(photos) if photos else None} atomic_write_text( @@ -355,6 +368,16 @@ def record_title_split(path: Path, title: str, photos: list[str] | None = None) ) +def record_title_split(path: Path, title: str, photos: list[str] | None = None) -> None: + _record_scoped(path, title, photos) + + +def record_title_removal( + path: Path, title: str, photos: list[str] | None = None +) -> None: + _record_scoped(path, title, photos) + + def dedupe_entries(entries: list[dict], splits: list[dict] | None = None) -> list[dict]: """Collapse same-normalized-title sightings unless their cues conflict — or unless the human declared them split (several physical copies): @@ -383,12 +406,28 @@ def dedupe_entries(entries: list[dict], splits: list[dict] | None = None) -> lis return result +def apply_title_removals(entries: list[dict], removals: list[dict]) -> list[dict]: + """Filter out sightings the human removed. Runs AFTER edits (records + key on the title as displayed when removal was clicked) and before + dedupe (so a removed sighting can't be absorbed into a survivor).""" + if not removals: + return entries + return [ + e + for e in entries + if not is_split( + normalize_title(e["title_raw"]), e.get("source_photos"), removals + ) + ] + + def rebuild_artifacts( raw_dir: Path, titles_path: Path, unidentified_path: Path, splits: list[dict] | None = None, edits: list[dict] | None = None, + removals: list[dict] | None = None, ) -> tuple[list[dict], dict[str, list[dict]]]: """Regenerate titles.json and unidentified.json from the per-photo raw cache. A raw file is either an object with titles/unidentified or a bare @@ -410,7 +449,10 @@ def rebuild_artifacts( photo = raw_file.name.removesuffix(".json") if data.get("unidentified"): unidentified[photo] = data["unidentified"] - deduped = dedupe_entries(apply_title_edits(entries, edits or []), splits) + entries = apply_title_removals( + apply_title_edits(entries, edits or []), removals or [] + ) + deduped = dedupe_entries(entries, splits) titles_path.parent.mkdir(parents=True, exist_ok=True) atomic_write_text( titles_path, json.dumps(deduped, indent=2, ensure_ascii=False) + "\n" @@ -429,6 +471,7 @@ def replay_titles(cfg: Config) -> None: photo-level splits and targeted edits can take hold.""" splits = load_title_splits(cfg.title_splits_path) edits = load_title_edits(cfg.title_edits_path) + removals = load_title_removals(cfg.title_removals_path) raw_dir = cfg.extract_raw_dir raw_photos = ( {f.name.removesuffix(".json") for f in raw_dir.glob("*.json")} @@ -444,7 +487,7 @@ def replay_titles(cfg: Config) -> None: # them would silently truncate the committed catalog if raw_photos and raw_photos >= known_photos: rebuild_artifacts( - raw_dir, cfg.titles_path, cfg.unidentified_path, splits, edits + raw_dir, cfg.titles_path, cfg.unidentified_path, splits, edits, removals ) return if not cfg.titles_path.exists(): @@ -456,7 +499,9 @@ def replay_titles(cfg: Config) -> None: exploded.extend({**entry, "source_photos": [p]} for p in photos) else: exploded.append(dict(entry)) - deduped = dedupe_entries(apply_title_edits(exploded, edits), splits) + deduped = dedupe_entries( + apply_title_removals(apply_title_edits(exploded, edits), removals), splits + ) atomic_write_text( cfg.titles_path, json.dumps(deduped, indent=2, ensure_ascii=False) + "\n" ) @@ -547,6 +592,7 @@ def run_extract( cfg.unidentified_path, load_title_splits(cfg.title_splits_path), load_title_edits(cfg.title_edits_path), + load_title_removals(cfg.title_removals_path), ) typer.echo(f"Wrote {len(deduped)} unique title(s) to {cfg.titles_path}.") if failed: diff --git a/src/bggpipe/review.py b/src/bggpipe/review.py index 30a7599..ec4f74b 100644 --- a/src/bggpipe/review.py +++ b/src/bggpipe/review.py @@ -270,13 +270,16 @@ class ReviewSession: title_raw: str, photos: list[str] | None = None, new_title: str | None = None, + even_vetoed: bool = False, ) -> int: """An edit invalidated these rows — the BGG match was made against the uncorrected read. Remove them so resolve re-queries with the fix; `photos` narrows the cull to one copy of a split title. Rows carrying a human veto (dedupe_veto) are never dropped — the match itself was human-vetted, and dropping would erase the veto; - a rename updates their title in place so they follow the entry.""" + a rename updates their title in place so they follow the entry. + `even_vetoed` is for title REMOVAL, where the human is explicitly + discarding the line, vetted or not.""" self.reload_if_changed() norm = normalize_title(title_raw) keep: list[dict] = [] @@ -288,7 +291,7 @@ class ReviewSession: ) if not targeted: keep.append(row) - elif row.get("dedupe_veto"): + elif row.get("dedupe_veto") and not even_vetoed: if new_title and row["title_raw"] != new_title: row["title_raw"] = new_title renamed += 1 diff --git a/src/bggpipe/templates/pages/catalog.html b/src/bggpipe/templates/pages/catalog.html index 67413bf..5314471 100644 --- a/src/bggpipe/templates/pages/catalog.html +++ b/src/bggpipe/templates/pages/catalog.html @@ -25,6 +25,7 @@ function editorRow(c) { + saving re-queues this title for resolve with the corrected data @@ -82,6 +83,19 @@ async function refresh() { document.getElementById("catbody").addEventListener("click", async e => { const cancel = e.target.closest("button.canceledit"); if (cancel) { EDITING = null; LAST = null; render(); refresh().catch(() => {}); return; } + const rm = e.target.closest("button.removetitle"); + if (rm) { + const f = rm.closest("form.editform"); + if (!confirm(`Remove "${f.dataset.title}" from the catalog? ` + + `Re-running extract won't bring it back — the removal is saved ` + + `in data/title_removals.json (delete its record there to undo).`)) return; + const res = await apiPost("/api/remove-title", { + title_raw: f.dataset.title, + source_photos: f.dataset.photos, + }); + if (res) { EDITING = null; LAST = null; refresh().catch(() => {}); } + return; + } const edit = e.target.closest("button.edit"); if (edit) { EDITING = EDITING === edit.dataset.key ? null : edit.dataset.key; diff --git a/src/bggpipe/webreview.py b/src/bggpipe/webreview.py index d8b3b74..06b5be2 100644 --- a/src/bggpipe/webreview.py +++ b/src/bggpipe/webreview.py @@ -42,6 +42,7 @@ from bggpipe.extract import ( is_split, load_title_splits, record_title_edit, + record_title_removal, record_title_split, replay_titles, ) @@ -171,6 +172,11 @@ class EditBody(BaseModel): language: str | None = None +class RemoveBody(BaseModel): + title_raw: str + source_photos: str = "" + + class RunBody(BaseModel): dry_run: bool = True # upload only; the safe direction is the default limit: int | None = None @@ -874,6 +880,38 @@ def create_app( replay_titles(cfg) return state() + @app.post("/api/remove-title") + def api_remove_title(body: RemoveBody) -> dict: + with lock: + revision["n"] += 1 + _refuse_if_rewriting() + freshen() + photos = [p for p in body.source_photos.split(";") if p] + entry = _find_entry(body.title_raw, body.source_photos) + norm = normalize_title(body.title_raw) + has_rows = any( + normalize_title(r["title_raw"]) == norm + and ( + not photos + or {p for p in r["source_photos"].split(";") if p} & set(photos) + ) + for r in session.rows + ) + # surplus rows without an entry (a split copy whose entry is + # gone) are removable too — rows alone are enough to act on + if entry is None and not has_rows: + raise HTTPException( + 404, "title not found — titles.json changed underneath?" + ) + # removal is the human explicitly discarding the line: veto'd + # rows go too, then the durable record, then the replay + session.drop_rows(body.title_raw, photos or None, even_vetoed=True) + record_title_removal( + cfg.title_removals_path, body.title_raw, photos or None + ) + replay_titles(cfg) + return state() + @app.post("/api/veto-merge") def api_veto_merge(body: VetoBody) -> dict: with lock: diff --git a/tests/test_extract.py b/tests/test_extract.py index f33aaa4..394d205 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -24,6 +24,7 @@ from bggpipe.extract import ( replay_titles, run_extract, ) +from bggpipe.normalize import normalize_title def _write_image(path, size=(400, 300), fmt="JPEG", color=(200, 30, 30)): @@ -234,6 +235,22 @@ def test_edits_chain_and_target_photos(): assert entries[1] == _entry("Wiz-War", "b.jpg") # untargeted copy untouched +def test_removals_filter_rebuilds_photo_scoped(): + from bggpipe.extract import apply_title_removals + + entries = [ + _entry("Not A Game", "a.jpg"), + _entry("Not A Game", "b.jpg"), + _entry("Catan", "a.jpg"), + ] + removals = [{"norm": normalize_title("Not A Game"), "photos": {"a.jpg"}}] + kept = apply_title_removals(entries, removals) + assert [(e["title_raw"], e["source_photos"]) for e in kept] == [ + ("Not A Game", ["b.jpg"]), # other sighting untouched + ("Catan", ["a.jpg"]), + ] + + def test_stores_roundtrip_and_replay_from_raw(tmp_path): cfg = Config(data_dir=tmp_path / "data", photos_dir=tmp_path / "photos") raw = cfg.extract_raw_dir diff --git a/tests/test_webreview.py b/tests/test_webreview.py index dc4ee6b..b5891f2 100644 --- a/tests/test_webreview.py +++ b/tests/test_webreview.py @@ -726,3 +726,43 @@ def test_single_photo_rowless_entry_is_not_splittable(tmp_path): if c["title_raw"] == "Fresh Off The Shelf" ) assert line["can_split"] is False + + +def test_remove_title_is_durable_and_drops_vetoed_rows(tmp_path): + cfg = make_cfg(tmp_path) + rows = read_matches(cfg.matches_path) + rows.append( + _row( + title_raw="Citadels", + match_status="approved", + bgg_id="478", + source_photos="shelf.jpg", + dedupe_veto="1", + ) + ) + write_matches(cfg.matches_path, rows) + web = TestClient(create_app(cfg, client=unauthorized_client(tmp_path))) + res = web.post( + "/api/remove-title", + json={"title_raw": "Citadels", "source_photos": "shelf.jpg"}, + ) + assert res.status_code == 200 + assert "Citadels" not in [c["title_raw"] for c in res.json()["catalog"]] + # removal is explicit: the veto'd row goes too (unlike edits) + assert not any(r["title_raw"] == "Citadels" for r in read_matches(cfg.matches_path)) + (record,) = json.loads(cfg.title_removals_path.read_text()) + assert record == {"title": "Citadels", "photos": ["shelf.jpg"]} + # durable: a fresh replay keeps it gone; the sibling title is untouched + from bggpipe.extract import replay_titles + + replay_titles(cfg) + titles = {e["title_raw"] for e in json.loads(cfg.titles_path.read_text())} + assert titles == {"Fresh Off The Shelf"} + # unknown titles still 404 + assert ( + web.post( + "/api/remove-title", + json={"title_raw": "No Such Game", "source_photos": "x.jpg"}, + ).status_code + == 404 + )