De-slop pass: dead plumbing out, comments stop narrating history

Eric asked whether AI residue had crept in since the last credibility
pass. It had, in two forms — both descendants of the audit's
"row-edit flow is collid-exact now" fix, which corrected upload.py but
not everyone who believed the old constraint:

The deferral scaffolding survived its own removal: build_queue still
returned a permanently-empty deferred list, run_upload still unpacked
it and carried a dead echo branch repeating the DISPROVEN claim ("the
row-edit flow can't target a collid"). And diff.py held a vestigial
guard on the same false premise, refusing to upgrade a versionless
copy whenever a sibling carried a version and telling the human to do
it by hand — the collid-exact editor makes that upgrade safe, so the
guard cost a real capability and its test now pins the opposite.

A comment audit (13 findings, verified line-by-line) cleaned the rest:
the webreview docstring counted six pages of seven and claimed
"localhost only" (--lan exists); the nav comment still said "catalog"
for the page renamed Titles; extract's docstring credited "Claude
vision" though any OpenAI-compatible endpoint dispatches; and five
comments narrated retired bugs ("the original bug", "why the old
guess quit after page one", "made the count go negative") — each
rewritten as the present-tense constraint or trimmed, per the
standing rule: comments state what the code can't show, never how it
got that way.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016jXZFSTZQKzAC8fqpWSz9g

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)

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:

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/<photo>.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/<photo>.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.

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 <a class="btn"> 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
     # <li>s carrying a thumbnail (the paging <li>s are not), and paging is
-    # an AngularJS <ul class="pagination"> of anchors — NOT buttons named
-    # "next", which is why the old guess quit after page one.
+    # an AngularJS <ul class="pagination"> 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:

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]:

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():

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