From e6d45011cc2909f43bda31a00e3a524c8a0e09c7 Mon Sep 17 00:00:00 2001 From: Eric Wagoner Date: Thu, 6 Aug 2026 00:30:13 -0400 Subject: [PATCH] Audit round 7, web + stages cluster: 13 more verified findings fixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The web layer's serialization story had three gaps: /api/run started a stage without the lock, so a decision mid-save could pass the rewrite guard and still be clobbered by the stage's full rewrite (now the start itself serializes); /api/photos accepted a replacement photo while extract was running, permanently pairing the new bytes with the old photo's reads (now refuses like every other mutation); and /api/queue read session rows lock-free and stale (now freshens under the lock). The localhost Host allowlist applied only to writes — a DNS-rebound page could read pipeline state and shelf photos with plain GETs; it now covers all methods (foreign-Origin reads still pass: without CORS headers a cross-origin page can't read the response anyway). Data-loss finds: the off-BGG edit form re-rendered from games.json, which only sees hand data after enrich — so a second save resubmitted pre-save blanks and cleared the first (the detail endpoint now overlays local_games.json live). The local key embeds the photo list, so a new sighting orphaned hand-written facts silently; enrich now migrates them when the title still matches exactly one line, and warns instead of ever dropping. research() left the previous game's version verdicts on the row, riding a stale version_id onto the next pick; it clears all four fields as reopen does. find_row now prefers the version-open sibling on duplicate keys, mirroring _adopt. Re-adding a removed hand-added title silently no-opped behind a 200 — it now rescinds the removal (an explicit undo), and a true duplicate add answers 409. Smaller: parse_search's dedupe collapsed same-id rows under DIFFERENT names, discarding the alternate-name row whose exact match downstream scoring needed (now collapses same-name only; research merges its ballot per game preferring exact evidence); rpgitems rank in their own family so their rank parsed null; the pipeline badge counted review-retired queue rows as pending; the catalog pairing cascade ran per-entry so a tier-3 claim could steal a sibling's exact row (now tier-by-tier across all entries, as resolve does); library cards render a lone player bound without "undefined" and the seats filter tolerates it; added_no_version reads "done · no version" instead of a bare green done. Every finding verified against the code before fixing; each fix carries a regression test. 337 tests. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016jXZFSTZQKzAC8fqpWSz9g --- src/bggpipe/enrich.py | 33 +++++++ src/bggpipe/extract.py | 33 ++++++- src/bggpipe/models.py | 24 +++-- src/bggpipe/review.py | 36 ++++++-- src/bggpipe/templates/pages/library.html | 8 +- src/bggpipe/templates/pages/queue.html | 2 + src/bggpipe/webreview.py | 109 +++++++++++++++++------ tests/test_models.py | 15 ++++ tests/test_review.py | 63 +++++++++++++ tests/test_web_dashboard.py | 9 +- tests/test_webreview.py | 85 ++++++++++++++++++ 11 files changed, 366 insertions(+), 51 deletions(-) diff --git a/src/bggpipe/enrich.py b/src/bggpipe/enrich.py index 48ad49c..5141b50 100644 --- a/src/bggpipe/enrich.py +++ b/src/bggpipe/enrich.py @@ -136,6 +136,39 @@ def run_enrich( games[key].update( {k: v for k, v in (hand.get(key) or {}).items() if v not in (None, "")} ) + # the key embeds the photo list, so a new sighting or a title edit + # strands hand data under a key no row produces anymore. Same + # normalized title + exactly one candidate = unambiguous: migrate. + # Anything else is reported, never silently dropped. + orphans = [k for k in hand if k.startswith("local:") and k not in local_keys] + migrated = {} + for old_key in orphans: + title_part = old_key.split(":", 2)[1] + candidates = [k for k in local_keys if k.split(":", 2)[1] == title_part] + if len(candidates) == 1 and candidates[0] not in hand: + new_key = candidates[0] + hand[new_key] = hand.pop(old_key) + migrated[old_key] = new_key + games[new_key].update( + {k: v for k, v in hand[new_key].items() if v not in (None, "")} + ) + else: + typer.echo( + f" warning: hand-written data for {old_key!r} matches " + "no current catalog line — the facts are safe in " + f"{cfg.local_games_path.name} but will not show in the " + "library until the key matches again" + ) + if migrated: + atomic_write_text( + cfg.local_games_path, + json.dumps(hand, indent=2, ensure_ascii=False, sort_keys=True) + "\n", + ) + for old_key, new_key in migrated.items(): + typer.echo( + f" migrated hand-written data {old_key!r} -> {new_key!r} " + "(photo set changed; same title)" + ) # prune keys no current target claims: a row whose version was approved # after a bare-key run (or was later rejected) must not leave an orphan diff --git a/src/bggpipe/extract.py b/src/bggpipe/extract.py index 099d9b6..e663403 100644 --- a/src/bggpipe/extract.py +++ b/src/bggpipe/extract.py @@ -403,14 +403,43 @@ def load_title_additions(path: Path) -> list[dict]: return _load_store(path) -def record_title_addition(path: Path, entry: dict) -> None: +def record_title_addition(path: Path, entry: dict) -> bool: + """Record a hand-added game. False = an addition with this normalized + title is already on file (nothing written — edit that line instead); + a silent True here would let the caller report success for a no-op.""" existing = load_title_additions(path) norm = normalize_title(entry["title_raw"]) if any(normalize_title(e["title_raw"]) == norm for e in existing): - return # already on file; edit the existing line instead + return False atomic_write_text( path, json.dumps([*existing, entry], indent=2, ensure_ascii=False) + "\n" ) + return True + + +def rescind_title_removal(path: Path, title: str) -> bool: + """Drop any UNSCOPED removal of this normalized title (a hand-added + line removed earlier). Re-adding a title those records would filter + from every rebuild is an explicit undo, not a conflict. Photo-scoped + removals stay: they veto specific sightings, not the game.""" + norm = normalize_title(title) + stored = _load_store(path) + kept = [ + item + for item in stored + if not ( + (isinstance(item, str) and normalize_title(item) == norm) + or ( + isinstance(item, dict) + and not item.get("photos") + and normalize_title(item["title"]) == norm + ) + ) + ] + if len(kept) == len(stored): + return False + atomic_write_text(path, json.dumps(kept, indent=2, ensure_ascii=False) + "\n") + return True def load_title_splits(path: Path) -> list[dict]: diff --git a/src/bggpipe/models.py b/src/bggpipe/models.py index 17d5608..8943a6d 100644 --- a/src/bggpipe/models.py +++ b/src/bggpipe/models.py @@ -109,14 +109,18 @@ def parse_search(xml_text: str) -> list[SearchResult]: type=item.get("type", "boardgame"), ) # a multi-type search lists an expansion TWICE — once per matched - # type; keep one entry, preferring the specific type so expansion - # tagging (the base-vs-expansion review guard) survives - if result.bgg_id in by_id: - seen = results[by_id[result.bgg_id]] + # type; collapse only SAME-NAME duplicates, preferring the specific + # type so expansion tagging (the base-vs-expansion review guard) + # survives. A duplicate under a DIFFERENT name stays: it may be the + # alternate name that exact-matches the query, and dropping it + # would silently downgrade the match to fuzzy. + dedupe_key = (result.bgg_id, result.name.casefold()) + if dedupe_key in by_id: + seen = results[by_id[dedupe_key]] if seen.type == "boardgame" and result.type != "boardgame": - results[by_id[result.bgg_id]] = result + results[by_id[dedupe_key]] = result continue - by_id[result.bgg_id] = len(results) + by_id[dedupe_key] = len(results) results.append(result) if skipped and results: warnings.warn( @@ -155,6 +159,8 @@ def parse_things(xml_text: str) -> list[ThingDetails]: for item in _root(xml_text).findall("item"): name = item.find("name[@type='primary']") rank_elem = item.find(".//ranks/rank[@name='boardgame']") + if rank_elem is None: # RPGGeek items rank in their own family + rank_elem = item.find(".//ranks/rank[@name='rpgitem']") versions = [ v for v_item in item.findall("versions/item") @@ -249,7 +255,11 @@ def parse_things_full(xml_text: str) -> list[dict]: "weight": _attr_float(ratings.find("averageweight")) if ratings is not None else None, - "rank": _attr_int(item.find(".//ranks/rank[@name='boardgame']")), + "rank": _attr_int( + item.find(".//ranks/rank[@name='boardgame']") + if item.find(".//ranks/rank[@name='boardgame']") is not None + else item.find(".//ranks/rank[@name='rpgitem']") + ), "users_owned": _attr_int(ratings.find("owned")) if ratings is not None else None, diff --git a/src/bggpipe/review.py b/src/bggpipe/review.py index 0b983cf..87f65dc 100644 --- a/src/bggpipe/review.py +++ b/src/bggpipe/review.py @@ -331,9 +331,17 @@ class ReviewSession: undecided = [ row for row in matches if row["match_status"] in UNDECIDED_MATCH_STATUSES ] - if undecided or matches: - return (undecided or matches)[0] - return None + if undecided: + return undecided[0] + # both siblings decided (two-edition duplicates): prefer the one + # whose VERSION is still open, as _adopt does — otherwise a stale + # row_ix would land a version pick on the already-decided sibling + version_open = [ + row for row in matches if row["version_status"] == "version_ambiguous" + ] + if version_open: + return version_open[0] + return matches[0] if matches else None def cues_for( self, title_raw: str, source_photos: str | None = None @@ -381,8 +389,9 @@ class ReviewSession: results = ( self.client.search(query, types) if types else self.client.search(query) ) - cands = [ - Candidate( + merged: dict[int, Candidate] = {} + for r in results: + cand = Candidate( bgg_id=r.bgg_id, name=r.name, year=r.year, @@ -390,8 +399,12 @@ class ReviewSession: exact=normalize_title(r.name) == normalize_title(query), fuzzy=0.0, ) - for r in results - ][:12] + prior = merged.get(r.bgg_id) + # one ballot line per game; an alternate-name row that exact- + # matches the query outranks the primary-name row for it + if prior is None or (cand.exact and not prior.exact): + merged[r.bgg_id] = cand + cands = list(merged.values())[:12] if cands: try: stats = { @@ -412,9 +425,16 @@ class ReviewSession: row["candidates_json"] = json.dumps( [c.as_json() for c in cands], ensure_ascii=False ) - # a fresh ballot supersedes any earlier verdict on this row + # a fresh ballot supersedes any earlier verdict on this row — + # including the VERSION verdicts, which belong to the old game: a + # surviving version_id would ride into the next pick and put the + # wrong game's edition on the collection entry row["bgg_id"] = "" row["bgg_name"] = "" + row["version_status"] = "" + row["version_id"] = "" + row["version_name"] = "" + row["version_candidates_json"] = "[]" self._save(row) return len(cands) diff --git a/src/bggpipe/templates/pages/library.html b/src/bggpipe/templates/pages/library.html index 5295077..41a8e8c 100644 --- a/src/bggpipe/templates/pages/library.html +++ b/src/bggpipe/templates/pages/library.html @@ -29,8 +29,9 @@ let KIND = ""; // "" = all; "boardgame" also covers expansions function gameCard(g) { const art = g.thumbnail || g.image; - const players = g.min_players - ? (g.min_players === g.max_players ? `${g.min_players}` : `${g.min_players}–${g.max_players}`) + " players" + const lo = g.min_players ?? g.max_players, hi = g.max_players ?? g.min_players; + const players = lo + ? (lo === hi ? `${lo}` : `${lo}–${hi}`) + " players" : ""; const time = g.playtime ? `${g.playtime} min` : ""; const weight = g.weight ? `weight ${g.weight.toFixed(1)}` : ""; @@ -88,7 +89,8 @@ function render() { g.version && g.version.name, ].filter(Boolean).join(" ").toLowerCase().includes(q)); if (seats) rows = rows.filter(g => - (g.min_players || 0) <= seats && seats <= (g.max_players || 0)); + (g.min_players || g.max_players || 0) <= seats + && seats <= (g.max_players || g.min_players || 0)); rows = [...rows].sort(SORTS[document.getElementById("libsort").value] || SORTS.name); document.getElementById("libcount").innerHTML = `${rows.length} of ${GAMES.length} game(s)` diff --git a/src/bggpipe/templates/pages/queue.html b/src/bggpipe/templates/pages/queue.html index 4508644..44de87a 100644 --- a/src/bggpipe/templates/pages/queue.html +++ b/src/bggpipe/templates/pages/queue.html @@ -15,6 +15,8 @@ function table(headers, rows) { // snapshots and never shrink as work completes function stateChip(r) { if (r.stale) return `retired`; + if (r.state === "done" && r.last_status === "added_no_version") + return `done · no version`; if (r.state === "done") return `done`; if (r.state === "failed") return `failed`; return `pending`; diff --git a/src/bggpipe/webreview.py b/src/bggpipe/webreview.py index f2db1ad..d2a0d98 100644 --- a/src/bggpipe/webreview.py +++ b/src/bggpipe/webreview.py @@ -50,6 +50,7 @@ from bggpipe.extract import ( record_title_removal, record_title_split, replay_titles, + rescind_title_removal, ) from bggpipe.fsio import atomic_write_bytes, atomic_write_text from bggpipe.jobs import JobRunner @@ -440,9 +441,18 @@ def create_app( {"detail": "cross-origin request refused"}, status_code=403 ) return await call_next(request) - if request.method not in ("GET", "HEAD", "OPTIONS") and ( - host not in ALLOWED_HOSTS - or (origin_host is not None and origin_host not in ALLOWED_HOSTS) + if public: + # the app's own assets: no user data, safe under any Host + return await call_next(request) + # Host, ALL methods: a DNS-rebound page reads GETs under a foreign + # Host. Origin, mutations only: a cross-origin page cannot READ a + # response without CORS headers, but its bodyless POSTs still + # execute — so foreign-Origin reads pass, foreign-Origin writes + # don't. + if host not in ALLOWED_HOSTS or ( + request.method not in ("GET", "HEAD", "OPTIONS") + and origin_host is not None + and origin_host not in ALLOWED_HOSTS ): typer.echo( f"refused {request.method} {request.url.path}: host {host!r}" @@ -612,26 +622,30 @@ def create_app( # displayed each other's rows (buttons and ballots swapped owners) claimed: set[int] = set() - def row_for(entry) -> dict | None: - same_title = rows_by_title.get(entry.title_raw, []) - entry_photos = set(entry.source_photos) + def photos_of(r: dict) -> set[str]: + return {p for p in r["source_photos"].split(";") if p} - def photos_of(r: dict) -> set[str]: - return {p for p in r["source_photos"].split(";") if p} - - for match in ( - lambda r: photos_of(r) == entry_photos, - lambda r: bool(photos_of(r) & entry_photos), - lambda r: True, - ): - for r in same_title: - if id(r) not in claimed and match(r): + # each tier runs across ALL entries before the next loosens — a + # per-entry cascade would let an unrelated entry's tier-3 "any row" + # claim steal the row its sibling matches exactly (run_resolve + # makes the same two-pass guarantee) + paired: dict[int, dict] = {} + for tier in ( + lambda r, photos: photos_of(r) == photos, + lambda r, photos: bool(photos_of(r) & photos), + lambda r, photos: True, + ): + for entry in session.titles: + if id(entry) in paired: + continue + for r in rows_by_title.get(entry.title_raw, []): + if id(r) not in claimed and tier(r, set(entry.source_photos)): claimed.add(id(r)) - return r - return None + paired[id(entry)] = r + break for entry in session.titles: - row = row_for(entry) + row = paired.get(id(entry)) # a split row's photo set is narrower than its entry's — show # the row's own photos for split copies catalog.append( @@ -792,11 +806,13 @@ def create_app( from bggpipe.upload import annotate_queue, stale_jobs - log = rows(cfg.upload_log_path) - to_add = rows(cfg.to_add_path) - to_update = rows(cfg.to_update_path) - # a review decision taken after the last diff retires a queued job - stale = stale_jobs(to_add + to_update, session.rows) + with lock: + freshen() + log = rows(cfg.upload_log_path) + to_add = rows(cfg.to_add_path) + to_update = rows(cfg.to_update_path) + # a review decision taken after the last diff retires a queued job + stale = stale_jobs(to_add + to_update, list(session.rows)) return { "to_add": [ {**r, "stale": stale.get(r.get("bgg_id", ""), "")} @@ -932,16 +948,34 @@ def create_app( game = library_entries().get(key) if game is None: raise HTTPException(404, "no such game in the library") + if key.startswith("local:"): + # overlay the hand-written store LIVE: the edit form re-renders + # from this payload, and pre-enrich values here would resubmit + # as blanks and silently clear what was just saved + game = { + **game, + **{ + k: v + for k, v in (_load_local_games().get(key) or {}).items() + if v not in (None, "", []) + }, + } return game def _pending(path: Path, action: str, log_rows: list[dict]) -> int: - from bggpipe.upload import annotate_queue + from bggpipe.upload import annotate_queue, stale_jobs if not path.exists(): return 0 with path.open(newline="") as f: rows = list(csv.DictReader(f)) - return sum(1 for r in annotate_queue(rows, action, log_rows) if not r["state"]) + # retired rows aren't pending: upload will (rightly) never run them + stale = stale_jobs(rows, session.rows) + return sum( + 1 + for r in annotate_queue(rows, action, log_rows) + if not r["state"] and r.get("bgg_id", "") not in stale + ) def _csv_count(path: Path) -> int: if not path.exists(): @@ -1041,8 +1075,12 @@ def create_app( ) else: fn = stages[stage] - if not jobs.start(stage, fn): - raise HTTPException(409, "a stage is already running — wait for it") + # under the lock: a decision mid-save either lands before the + # stage's initial read or sees the running job and 409s — never + # silently clobbered by the stage's full rewrite + with lock: + if not jobs.start(stage, fn): + raise HTTPException(409, "a stage is already running — wait for it") return jobs.snapshot() @app.get("/api/job") @@ -1066,6 +1104,10 @@ def create_app( async def api_photos(files: list[UploadFile]) -> dict: saved = [] batch: set[str] = set() + with lock: + # replacing a photo mid-extract would pair the NEW bytes with + # the OLD photo's reads forever (extract skips existing raws) + _refuse_if_rewriting() cfg.photos_dir.mkdir(parents=True, exist_ok=True) for upload_file in files: name = Path(upload_file.filename or "").name # strips any path @@ -1316,7 +1358,16 @@ def create_app( "art_notes": "", "source_photos": [], } - record_title_addition(cfg.title_additions_path, entry) + # an earlier "remove" of this hand-added title filters it out + # of every rebuild: re-adding is an explicit undo of that + rescinded = rescind_title_removal(cfg.title_removals_path, title) + added = record_title_addition(cfg.title_additions_path, entry) + if not added and not rescinded: + raise HTTPException( + 409, + f"{title!r} is already on the hand-added list — edit " + "that line on the Titles page instead", + ) replay_titles(cfg) return state() diff --git a/tests/test_models.py b/tests/test_models.py index c3981c6..c4b93b4 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -173,3 +173,18 @@ def test_parse_things_full_reads_rpggeek_link_vocabulary(): assert game["categories"] == ["Modern", "Core Rules (min needed to play)"] assert game["mechanics"] == ["Card Play"] assert game["producers"] == ["Someone"] + + +def test_parse_search_keeps_alternate_name_duplicates(): + """The same id listed under two NAMES is evidence, not noise — the + alternate may be the one that exact-matches the query.""" + xml = """ + + + + + + + """ + results = parse_search(xml) + assert [r.name for r in results] == ["Dragones Y Mazmorras", "Dungeons & Dragons"] diff --git a/tests/test_review.py b/tests/test_review.py index 24997ad..0c29af7 100644 --- a/tests/test_review.py +++ b/tests/test_review.py @@ -698,3 +698,66 @@ def test_research_can_target_rpggeek_explicitly(tmp_path): assert row["bgg_id"] == "" with pytest.raises(ValueError, match="needs some text"): session.research(row, " ") + + +def test_find_row_prefers_the_version_open_sibling(tmp_path): + """Two decided duplicate rows: a version pick with a stale row_ix must + land on the sibling whose VERSION is still open, not overwrite the + other's earlier human decision (mirrors _adopt).""" + cfg = _setup( + tmp_path, + [ + _row( + title_raw="Wiz-War", + match_status="approved", + bgg_id="589", + version_status="version_approved", + version_id="1", + ), + _row( + title_raw="Wiz-War", + match_status="approved", + bgg_id="589", + version_status="version_ambiguous", + ), + ], + ) + session = ReviewSession( + cfg, console=quiet_console(), input_fn=scripted(), client=fixture_client() + ) + row = session.find_row("Wiz-War", "hand-typed-test-list", row_ix=None) + assert row["version_status"] == "version_ambiguous" + + +def test_research_clears_the_previous_games_version(tmp_path): + """A fresh ballot supersedes EVERYTHING about the old match: a + surviving version_id would put the old game's edition on whatever the + human picks next.""" + cfg = _setup( + tmp_path, + [ + _row( + title_raw="Citadels", + match_status="approved", + bgg_id="478", + version_status="version_approved", + version_id="99999", + version_name="Old Game's Edition", + ) + ], + ) + session = ReviewSession( + cfg, console=quiet_console(), input_fn=scripted(), client=fixture_client() + ) + # stats batch for these results isn't recorded; decoration may fail + session.client = BGGClient( + cache_dir=FIXTURES, + transport=httpx.MockTransport( + lambda req: httpx.Response(401, text="Unauthorized") + ), + ) + row = session.rows[0] + session.research(row, "Citadels") + assert row["version_status"] == "" + assert row["version_id"] == "" and row["version_name"] == "" + assert row["version_candidates_json"] == "[]" diff --git a/tests/test_web_dashboard.py b/tests/test_web_dashboard.py index 6cb9abb..5c50b42 100644 --- a/tests/test_web_dashboard.py +++ b/tests/test_web_dashboard.py @@ -633,10 +633,15 @@ def test_pipeline_reports_badge_fields(tmp_path): }, ], ) - cfg.to_add_path.write_text("bgg_id,bgg_name\n1,X\n2,Y\n") + # queue rows must be ENDORSED by matches.csv or they count as retired + cfg.to_add_path.write_text("bgg_id,bgg_name,version_id\n13,Catan,1\n") p = _app(cfg).get("/api/pipeline").json() assert p["pending_review"] == 2 # one match + one edition decision - assert p["to_add"] == 2 # header excluded + assert p["to_add"] == 1 # header excluded + # a queue row review no longer backs is not pending work + cfg.to_add_path.write_text("bgg_id,bgg_name,version_id\n13,Catan,1\n99,Gone,\n") + p = _app(cfg).get("/api/pipeline").json() + assert p["to_add"] == 1 def test_photo_detail_page_serves_with_photos_nav_active(tmp_path): diff --git a/tests/test_webreview.py b/tests/test_webreview.py index 2016ca4..ee8506b 100644 --- a/tests/test_webreview.py +++ b/tests/test_webreview.py @@ -1344,3 +1344,88 @@ def test_local_game_notes_and_art_round_trip(tmp_path): == 400 ) assert web.post(f"/api/local-game/{key}", json={"year": "19x8"}).status_code == 400 + + +def test_research_endpoint_contract(tmp_path): + """The web UI's manual-search button: 200 replaces the ballot, blank + query 400s, BGG-down 502s, and `types` reaches the client (the + RPGGeek toggle is the whole point).""" + cfg = make_cfg(tmp_path) + rows = read_matches(cfg.matches_path) + rows.append(_row(title_raw="MYSTERY BOX", match_status="unmatched")) + write_matches(cfg.matches_path, rows) + + seen_queries = [] + + class _Recorder(BGGClient): + def search(self, query, types=None): + seen_queries.append((query, types)) + return [] + + client = _Recorder(cache_dir=tmp_path / "no_cache") + web = TestClient(create_app(cfg, client=client)) + body = { + "title_raw": "MYSTERY BOX", + "source_photos": "shelf.jpg", + "query": "Alice is Missing", + "types": "rpgitem", + } + assert web.post("/api/research", json=body).status_code == 200 + assert ("Alice is Missing", "rpgitem") in seen_queries + + assert web.post("/api/research", json={**body, "query": " "}).status_code == 400 + + down = TestClient(create_app(cfg, client=unauthorized_client(tmp_path))) + assert down.post("/api/research", json=body).status_code == 502 + + +def test_corrupt_local_games_store_returns_500_not_silence(tmp_path): + web, cfg = make_client(tmp_path) + cfg.local_games_path.write_text("{torn") + res = web.post("/api/local-game/local:x:y.jpg", json={"name": "X"}) + assert res.status_code == 500 + assert "local_games.json" in res.json()["detail"] + + +def test_local_game_detail_reflects_saved_facts_before_enrich(tmp_path): + """The edit form re-renders from the detail payload; serving pre-save + values there would resubmit as blanks and clear the store.""" + cfg = make_cfg(tmp_path) + rows = read_matches(cfg.matches_path) + rows.append( + _row(title_raw="Homebrew Game", match_status="local", source_photos="s.jpg") + ) + write_matches(cfg.matches_path, rows) + key = "local:homebrew game:s.jpg" + cfg.games_path.write_text( + json.dumps({key: {"name": "Homebrew Game", "type": "localgame"}}) + ) + web = TestClient(create_app(cfg, client=unauthorized_client(tmp_path))) + web.post(f"/api/local-game/{key}", json={"designers": "A Friend", "year": "1998"}) + detail = web.get(f"/api/library/{key}").json() + assert detail["designers"] == ["A Friend"] # live, before any enrich + assert detail["year"] == 1998 + + +def test_readding_a_removed_hand_added_title_rescinds_the_removal(tmp_path): + """add → remove → add again must resurrect the line, not silently + no-op behind a success response.""" + web, cfg = make_client(tmp_path) + add = { + "title": "Homebrew Quest", + "publisher": "", + "edition": "", + "year": "", + "language": "", + } + assert web.post("/api/add-title", json=add).status_code == 200 + # a second identical add is a refused no-op, not a silent success + assert web.post("/api/add-title", json=add).status_code == 409 + state = web.post( + "/api/remove-title", + json={"title_raw": "Homebrew Quest", "source_photos": ""}, + ) + assert state.status_code == 200 + assert web.post("/api/add-title", json=add).status_code == 200 + titles = json.loads(cfg.titles_path.read_text()) + assert any(t["title_raw"] == "Homebrew Quest" for t in titles)