diff --git a/src/bggpipe/diff.py b/src/bggpipe/diff.py index ae49db9..d9e61b7 100644 --- a/src/bggpipe/diff.py +++ b/src/bggpipe/diff.py @@ -183,25 +183,13 @@ def compute_diff(rows: list[dict], collection: list[CollectionItem]) -> DiffResu else: leftover.append(row) - # 1b — versionless copies get upgraded. Guard: upload's row-edit flow - # targets rows by game NAME (a collid can't drive the UI), so an update - # is only safe when EVERY copy of the game is versionless — otherwise - # the browser could open the versioned copy and overwrite it. + # 1b — versionless copies get upgraded. The update flow addresses the + # copy by collid and the edition by radio value, so a versioned + # sibling copy is never at risk. still_left: list[dict] = [] for row in leftover: bgg_id = int(row["bgg_id"]) versionless = [c for c in unconsumed(bgg_id) if c.version_id is None] - any_versioned = any(c.version_id is not None for c in by_object.get(bgg_id, [])) - if versionless and any_versioned: - consumed_collids.add(versionless[0].coll_id) - result.already_owned.append(row["title_raw"]) - result.disagreements.append( - f"{row['title_raw']}: a versionless copy could take version " - f"{row['version_name']!r}, but another copy already carries " - "a version — set it by hand on BGG (the automated row edit " - "can't safely target a specific copy)" - ) - continue if versionless: target = versionless[0] consumed_collids.add(target.coll_id) diff --git a/src/bggpipe/enrich.py b/src/bggpipe/enrich.py index 5141b50..e335f40 100644 --- a/src/bggpipe/enrich.py +++ b/src/bggpipe/enrich.py @@ -190,8 +190,8 @@ def run_enrich( ) # count the two populations separately: local entries are built from - # photo reads and have no target, so folding them into the API tally - # made "already present or waiting" go negative + # photo reads and have no fetch target, so folding them into the API + # tally would drive "already present or waiting" negative waiting = len(targets) - updated parts = [f"{updated} fetched from BGG"] if local_keys: diff --git a/src/bggpipe/extract.py b/src/bggpipe/extract.py index e663403..ec9aa1e 100644 --- a/src/bggpipe/extract.py +++ b/src/bggpipe/extract.py @@ -1,7 +1,8 @@ """Stage 1 — extract game titles + edition cues from shelf photos. -Each photo is sent to Claude vision once and the raw result is cached under -data/extract_raw/.json (making re-runs free and --only targeted). +Each photo is sent to the configured vision model once and the raw result +is cached under data/extract_raw/.json (making re-runs free and +--only targeted). titles.json is rebuilt from all raw files on every run, deduping identical titles across photos unless their edition cues conflict — conflicting cues mean two different editions on the shelf, which stay separate entries. diff --git a/src/bggpipe/upload.py b/src/bggpipe/upload.py index 3e17dad..f529143 100644 --- a/src/bggpipe/upload.py +++ b/src/bggpipe/upload.py @@ -235,10 +235,9 @@ def build_queue( log_rows: list[dict], *, retry_failed: bool = False, -) -> tuple[list[UploadJob], int, int, list[UploadJob]]: +) -> tuple[list[UploadJob], int, int]: """Turn the diff outputs into pending jobs, minus work the log says is - done. Returns (jobs, skipped_done, skipped_failed, deferred) — deferred - being same-game updates held for a later run. + done. Returns (jobs, skipped_done, skipped_failed). Completions are COUNTED per key, not looked up: two vetoed duplicate copies share a key, and one logged success must complete exactly one @@ -282,7 +281,6 @@ def build_queue( jobs: list[UploadJob] = [] skipped_done = skipped_failed = 0 - deferred: list[UploadJob] = [] # kept for callers; nothing defers now seen: Counter[tuple[str, str, str]] = Counter() queued_versions: dict[str, set[str]] = {} for job in candidates: @@ -325,7 +323,7 @@ def build_queue( # the copy by collid and the edition by radio value, so a # second update cannot reopen what the first just saved jobs.append(job) - return jobs, skipped_done, skipped_failed, deferred + return jobs, skipped_done, skipped_failed def _scrub(text: str) -> str: @@ -395,8 +393,7 @@ class PlaywrightUploader: self._page.goto(url, wait_until="domcontentloaded") # BGG's Sign In control is an with NO href, so it has - # no implicit link role — a role="link" query never matched it in any - # state, and "no Sign In link" silently meant "already logged in". + # no implicit link role — role-based queries cannot find it. _SIGN_IN = 'a:has-text("Sign In"), button:has-text("Sign In")' _SIGNED_IN = ':text-is("Sign Out"), :text-is("Log Out")' @@ -404,8 +401,8 @@ class PlaywrightUploader: """True = signed out. Waits for the header to prove ONE state or the other: BGG hydrates it after domcontentloaded, so an early read finds NEITHER control — and any check resting on a single absence - (the original bug) then guesses, silently, in the unsafe direction. - Undeterminable state raises rather than assumes.""" + guesses, silently, in the unsafe direction. Undeterminable state + raises rather than assumes.""" page = self._page deadline = time.monotonic() + 30 while time.monotonic() < deadline: @@ -483,8 +480,7 @@ class PlaywrightUploader: # Verified 2026-08-06 against the live picker: version rows are the #
  • s carrying a thumbnail (the paging
  • s are not), and paging is - # an AngularJS
      of anchors — NOT buttons named - # "next", which is why the old guess quit after page one. + # an AngularJS
        of anchors, not buttons. _VERSION_ROWS = "li:has(.summary-item-thumbnail)" # Paging controls render TWICE — a desktop set and a mobile set marked # visible-xs-* — so every one of these selectors matches hidden nodes @@ -931,7 +927,7 @@ def run_upload( for bgg_id, why in stale.items(): typer.echo(f" {bgg_id}: {why}") - jobs, skipped_done, skipped_failed, deferred = build_queue( + jobs, skipped_done, skipped_failed = build_queue( to_add, to_update, log_rows, retry_failed=retry_failed ) if limit is not None: @@ -943,12 +939,6 @@ def run_upload( f" (skipping {skipped_done} already done, {skipped_failed} " "previously failed — use --retry-failed)" ) - if deferred: - typer.echo( - f"{len(deferred)} version update(s) deferred: one update per game " - "per run (the row-edit flow can't target a collid) — re-run " - "upload after --verify confirms this batch." - ) results: list[dict] = [] if dry_run: diff --git a/src/bggpipe/webreview.py b/src/bggpipe/webreview.py index 73eff65..adb276a 100644 --- a/src/bggpipe/webreview.py +++ b/src/bggpipe/webreview.py @@ -1,9 +1,10 @@ -"""The local web app: six pages in a shared sidebar shell. +"""The local web app: one page per pipeline concern in a shared sidebar shell. Pipeline (/), Photos, Titles, Review, Queue, Library, and Help — rendered server-side from templates/shell.html plus one fragment per page, with static/app.css as the design system and static/app.js as shared client -plumbing. No template engine and no build step; served on localhost only. +plumbing. No template engine and no build step; served on localhost by default — +--lan opens it to the network behind a per-device access key. All decision logic and matches.csv writes go through ReviewSession — this module is purely an interface. Reshoot work-orders come from @@ -272,7 +273,7 @@ def _default_stages(cfg: Config) -> dict[str, Callable[..., object]]: # (href, page-name, label, badge-name) — badge names match app.js # ordered as the real workflow runs: photograph, proofread the reads -# (catalog), resolve + decide (review), then what ships (queue, library) +# (titles), resolve + decide (review), then what ships (queue, library) NAV_PAGES = ( ("/", "pipeline", "Pipeline", ""), ("/photos", "photos", "Photos", "photos"), @@ -620,9 +621,9 @@ def create_app( } # photo-overlap pairing first, positional fallback — the same rule - # as run_resolve. Positional-only broke the moment an edited copy's - # row was recreated at the END of matches.csv: same-title lines - # displayed each other's rows (buttons and ballots swapped owners) + # as run_resolve. Purely positional pairing mis-pairs same-title + # lines whenever an edited copy's row is recreated at the END of + # matches.csv: buttons and ballots would swap owners claimed: set[int] = set() def photos_of(r: dict) -> set[str]: diff --git a/tests/test_diff.py b/tests/test_diff.py index 95cceb1..542bfc4 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -192,13 +192,14 @@ def test_earlier_disagreement_cannot_steal_a_later_rows_exact_match(): assert result.disagreements == [] -def test_update_withheld_when_another_copy_is_versioned(): - # upload's row edit targets by NAME: an update is only safe when every - # copy is versionless, else it could overwrite the versioned copy +def test_versionless_copy_upgrades_even_beside_a_versioned_sibling(): + # the update flow is collid-exact, so the versioned copy (collid 900) + # is never at risk: the versionless one (collid 901) takes the upgrade rows = [_match("Catan", "13", vstatus="version_auto", vid="5", vname="5th")] result = compute_diff(rows, [_item(13, 900, version_id=7), _item(13, 901)]) - assert result.to_update == [] - assert "set it by hand" in result.disagreements[0] + (update,) = result.to_update + assert update["collid"] == "901" and update["version_id"] == "5" + assert result.disagreements == [] def test_bare_row_does_not_steal_versionless_copy_from_confident_update(): diff --git a/tests/test_upload.py b/tests/test_upload.py index 255cc1f..758931d 100644 --- a/tests/test_upload.py +++ b/tests/test_upload.py @@ -116,7 +116,7 @@ class FakeUploader: def test_build_queue_skips_logged_successes(): - jobs, done, failed, _ = build_queue( + jobs, done, failed = build_queue( [_add_row(bgg_id="1"), _add_row(bgg_id="2", name="Catan")], [_update_row(collid="9")], [ @@ -132,7 +132,7 @@ def test_build_queue_skips_logged_successes(): def test_build_queue_second_copy_is_a_distinct_job(): # Same game, different version: a separate physical copy, so a # logged add of one version must not swallow the other. - jobs, done, _, _ = build_queue( + jobs, done, _ = build_queue( [ _add_row(bgg_id="1", version_id="10", version_name="First ed."), _add_row(bgg_id="1", version_id="11", version_name="Second ed."), @@ -146,11 +146,9 @@ def test_build_queue_second_copy_is_a_distinct_job(): def test_build_queue_failures_need_retry_flag(): log = [_log_row(action="add", bgg_id="1", status="failed")] - jobs, _, skipped, _ = build_queue([_add_row(bgg_id="1")], [], log) + jobs, _, skipped = build_queue([_add_row(bgg_id="1")], [], log) assert jobs == [] and skipped == 1 - jobs, _, skipped, _ = build_queue( - [_add_row(bgg_id="1")], [], log, retry_failed=True - ) + jobs, _, skipped = build_queue([_add_row(bgg_id="1")], [], log, retry_failed=True) assert len(jobs) == 1 and skipped == 0 @@ -160,7 +158,7 @@ def test_build_queue_latest_log_entry_wins(): _log_row(action="add", bgg_id="1", status="failed"), _log_row(action="add", bgg_id="1", status="added"), ] - jobs, done, _, _ = build_queue([_add_row(bgg_id="1")], [], log, retry_failed=True) + jobs, done, _ = build_queue([_add_row(bgg_id="1")], [], log, retry_failed=True) assert jobs == [] and done == 1