From 6ab4a836b4e4d74bff065df5273ff1ea1c5e72c7 Mon Sep 17 00:00:00 2001 From: Eric Wagoner Date: Sun, 9 Aug 2026 14:02:00 -0400 Subject: [PATCH] Audit round 9 lands: the location layer keeps its promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five blind reviewers over the day-old shelves layer, ~28 verified findings — plus Eric's screenshot catching the biggest one live: the generic .card is flex (built for review's photo-beside-ballot layout), so unit headers shared a row with their grids and unshelved rows flowed horizontally off the page. Shelf cards are now .card.stack. The model fixes. Containment chains resolve recursively with a cycle guard — minis inside an insert inside a big box live where the big box does, instead of vanishing from every list; the stored-in endpoint walks the whole chain when refusing cycles, and clears the newly contained game's own shelf spot (one box must never consume capacity in two openings). Contained games are listed residents but occupy no shelf space: only physical boxes are stacked and fit-checked — the web report now agrees with the dims report about the same opening. A location pointing at a vanished opening (hand-edited or reverted store) SURFACES as unshelved with a "shelf gone" chip in the app and counts as homeless in the CLI, instead of hiding the game from every list while the page declares everything has a home. Honest edges. Opening dimensions are all-or-none everywhere (a half-sized opening silently became limitless; the CLI report crashed formatting it); a second grid on a unit continues the row letters so labels stay unique and label-addressed CSV imports keep working, and row letters survive past Z; CSV re-imports preserve hand-entered notes; the ambiguity reject names the fix that actually works; corrupt furniture/locations stores speak a 500 instead of a raw traceback; the dims help text stops saying Kallax; DIM_AXES gets one home in models.py instead of three drifting copies; the new stores join CLAUDE.md's commit registry. The page behaves. Custom-dims fields hide unless the custom preset is chosen (typed values were silently discarded); the sheet is a real dialog (role, aria-modal, Escape, focus return, one layer at a time); backdrop close requires press AND release on the backdrop (a text- selection drag out of the search box no longer dismisses); refresh goes through changeGate and stops wiping the search mid-interaction; the prompt() chain is an inline per-unit form with client-side all-or-none validation; unit-create only toasts success after the openings actually land, recovers from its own half-failures, and guards against double-submit (a click retried against the re-rendering DOM built a second grid — caught live in a Playwright run); warnings speak (aria-labels on ⚠ and overfull in the fill bar's label); unmeasured boxes are visible in cells and sheet rows; the library's unshelved filter matches the Shelves page's definition; reorder buttons have names; the detail locform wraps at phone width. Eight new regression tests from the seats' sketches; 380 total. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016jXZFSTZQKzAC8fqpWSz9g --- CLAUDE.md | 4 +- docs/guide.md | 10 +- src/bggpipe/cli.py | 2 +- src/bggpipe/dims.py | 10 +- src/bggpipe/enrich.py | 9 +- src/bggpipe/models.py | 5 + src/bggpipe/shelves.py | 54 +++-- src/bggpipe/static/app.css | 13 +- src/bggpipe/templates/pages/library.html | 4 +- src/bggpipe/templates/pages/shelves.html | 164 +++++++++++---- src/bggpipe/webreview.py | 105 +++++++-- tests/test_shelves.py | 257 +++++++++++++++++++++++ 12 files changed, 549 insertions(+), 88 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 97050a6..2288b26 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,11 +50,11 @@ Full design lives in `docs/spec.md` (read it before changing pipeline semantics) - 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), `data/title_edits.json` (corrected reads/cues, applied before dedupe on every titles.json rebuild), `data/title_removals.json` (lines removed from the catalog — filtered out of every rebuild; delete the record to undo), `data/title_additions.json` (games added without a photo — joined into every rebuild; a later photo sighting dedupe-merges with them), and `data/local_games.json` + `data/local_art/` (hand-written facts and a cover photo for off-BGG games — the ONLY source for them, merged over the photo reads by enrich) 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). +- **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), `data/title_removals.json` (lines removed from the catalog — filtered out of every rebuild; delete the record to undo), `data/title_additions.json` (games added without a photo — joined into every rebuild; a later photo sighting dedupe-merges with them), `data/local_games.json` + `data/local_art/` (hand-written facts and a cover photo for off-BGG games — the ONLY source for them, merged over the photo reads by enrich), and `data/furniture.json` + `data/locations.json` (the owner's shelf layout and each game's physical spot — local-only, never BGG's business) 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). RPGGeek items carry their OWN link types (`rpgdesigner`, `rpgpublisher`, `rpggenre`, `rpgcategory`, `rpgmechanic`) — a board-game-only parser silently returns nothing for them. 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/title_removals.json`, `data/title_additions.json`, `data/local_games.json`, `data/local_art/`, `data/games.json`, `data/STUB_DATA.marker` (while it applies), and the collection snapshot XMLs. Never commit `data/bgg_cache/`, `data/extract_raw/`, `photos/`, `data/.lan_key`, 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/title_additions.json`, `data/local_games.json`, `data/local_art/`, `data/furniture.json`, `data/locations.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/`, `data/.lan_key`, Playwright storage state, or `.env`. diff --git a/docs/guide.md b/docs/guide.md index 185027d..fcb6469 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -144,11 +144,11 @@ BGG's `0` means "never entered", not zero inches). Then: bggpipe dims ``` -reports coverage, the ten biggest footprints, and a fit check against an IKEA -Kallax cube (13.2" × 13.2" opening, 15.4" deep) — listing every box that fits -in **no** orientation, and every game whose dimensions can't be verified, -because can't-verify is not the same as fits. Trusted dimensions also show on -each game's Library detail page. +reports coverage, the ten biggest footprints, and per-opening fit checks +against your own furniture (see *Where everything lives*) — capacity, +overfull warnings, boxes that fit in **no** orientation, and every game whose +dimensions can't be verified, because can't-verify is not the same as fits. +Trusted dimensions also show on each game's Library detail page. ## Publishing your library on your own site diff --git a/src/bggpipe/cli.py b/src/bggpipe/cli.py index ef99312..6796b6e 100644 --- a/src/bggpipe/cli.py +++ b/src/bggpipe/cli.py @@ -212,7 +212,7 @@ def shelve( @app.command() def dims(config: ConfigOpt = None) -> None: - """Shelf-space report: box sizes, biggest footprints, Kallax fit.""" + """Shelf-space report: box sizes, biggest footprints, per-opening fit.""" from bggpipe.dims import run_dims_report cfg = load_config(config) diff --git a/src/bggpipe/dims.py b/src/bggpipe/dims.py index 12cb61f..21435fc 100644 --- a/src/bggpipe/dims.py +++ b/src/bggpipe/dims.py @@ -14,6 +14,7 @@ import json import typer from bggpipe.config import Config +from bggpipe.models import DIM_AXES from bggpipe.shelves import ( load_furniture, load_locations, @@ -21,7 +22,7 @@ from bggpipe.shelves import ( opening_report, ) -_AXES = ("width_in", "length_in", "depth_in") +_AXES = DIM_AXES def _footprint(dims: dict) -> float: @@ -106,10 +107,11 @@ def run_dims_report(cfg: Config) -> dict: typer.echo("\nOpenings:") for oid, opening in openings.items(): report = opening_report(opening, by_opening.get(oid, [])) + sized = all(opening.get(k) for k in ("width_in", "height_in", "depth_in")) dims_note = ( f"{opening['width_in']:g}×{opening['height_in']:g}" f"×{opening['depth_in']:g} in" - if opening.get("width_in") + if sized else "no size limits" ) line = ( @@ -130,7 +132,9 @@ def run_dims_report(cfg: Config) -> dict: entry.get("name") or "?" for key, entry in games.items() if not entry.get("stored_in") - and not (locations.get(key) or {}).get("opening_id") + # an assignment to a vanished opening is homeless too — a + # truthy id must not hide a game from the count + and (locations.get(key) or {}).get("opening_id") not in openings ] if homeless: typer.echo(f"\n{len(homeless)} game(s) unshelved.") diff --git a/src/bggpipe/enrich.py b/src/bggpipe/enrich.py index 6f7acbe..c1ef073 100644 --- a/src/bggpipe/enrich.py +++ b/src/bggpipe/enrich.py @@ -27,13 +27,18 @@ from bggpipe.bgg_client import ( ) from bggpipe.config import Config from bggpipe.fsio import atomic_write_text -from bggpipe.models import is_confident_version, is_recognized, parse_version_dims +from bggpipe.models import ( + DIM_AXES, + is_confident_version, + is_recognized, + parse_version_dims, +) from bggpipe.normalize import normalize_title from bggpipe.resolve import load_titles, read_matches BATCH_SIZE = 20 -_AXES = ("width_in", "length_in", "depth_in") +_AXES = DIM_AXES _AGREE_IN = 0.5 # per-axis tolerance for calling versionless dims unanimous diff --git a/src/bggpipe/models.py b/src/bggpipe/models.py index d41ad69..5a1777e 100644 --- a/src/bggpipe/models.py +++ b/src/bggpipe/models.py @@ -23,6 +23,11 @@ PENDING_MATCH_STATUSES = ("ambiguous", "unmatched") UNDECIDED_MATCH_STATUSES = ("ambiguous", "unmatched", "merged") +# the write/read contract for entry["dims"]: enrich writes these keys, +# shelves and dims consume them +DIM_AXES = ("width_in", "length_in", "depth_in") + + def is_recognized(row: dict) -> bool: """A row that reaches diff/enrich: matched and carrying a real id.""" return row["match_status"] in RECOGNIZED_MATCH_STATUSES and bool(row["bgg_id"]) diff --git a/src/bggpipe/shelves.py b/src/bggpipe/shelves.py index d872e51..25422c8 100644 --- a/src/bggpipe/shelves.py +++ b/src/bggpipe/shelves.py @@ -26,9 +26,10 @@ import typer from bggpipe.config import Config from bggpipe.fsio import atomic_write_text +from bggpipe.models import DIM_AXES from bggpipe.normalize import normalize_title -_AXES = ("width_in", "length_in", "depth_in") +_AXES = DIM_AXES _SLUG = re.compile(r"[^a-z0-9]+") @@ -136,32 +137,49 @@ def effective_location( key: str, entry: dict, games: dict[str, dict], locations: dict[str, dict] ) -> tuple[str | None, str, str | None]: """(opening_id, note, inherited_from_key). A contained game inherits - its container's location; everything else reads locations.json.""" - container_id = entry.get("stored_in") - if container_id: - for other_key, other in games.items(): - if str(other.get("bgg_id")) == str(container_id): - loc = locations.get(other_key, {}) - return loc.get("opening_id"), loc.get("note", ""), other_key - return None, "", None - loc = locations.get(key, {}) - return loc.get("opening_id"), loc.get("note", ""), None + its container's location through the WHOLE chain — minis inside an + insert inside a big box live wherever the big box does. A cycle (which + the endpoints refuse, but hand-edited stores can hold) terminates as + unlocated rather than looping.""" + visited = {key} + via: str | None = None + current_key, current = key, entry + while current.get("stored_in"): + container = next( + ( + (k, e) + for k, e in games.items() + if str(e.get("bgg_id")) == str(current["stored_in"]) + ), + None, + ) + if container is None or container[0] in visited: + return None, "", via + current_key, current = container + visited.add(current_key) + via = current_key + loc = locations.get(current_key, {}) + return loc.get("opening_id"), loc.get("note", ""), via if via != key else None def opening_report(opening: dict, games_here: list[tuple[str, dict]]) -> dict: """Capacity picture for one opening: fill (stacked thickness vs - interior height), misfits, and how many boxes are unmeasured.""" - thicknesses = [stack_thickness(e) for _, e in games_here] + interior height), misfits, and how many boxes are unmeasured. Games + stored INSIDE another box are listed residents but occupy no shelf + space of their own — only their container is stacked and fit-checked, + the same stance the dims report takes.""" + physical = [(k, e) for k, e in games_here if not e.get("stored_in")] + thicknesses = [stack_thickness(e) for _, e in physical] stacked = sum(t for t in thicknesses if t) unmeasured = sum(1 for t in thicknesses if t is None) height = opening.get("height_in") misfits = [ entry.get("name") or "?" - for _, entry in games_here + for _, entry in physical if fits_opening(entry, opening) is False ] return { - "games": len(games_here), + "games": len(physical), "stacked_in": round(stacked, 2), "unmeasured": unmeasured, "fill": round(stacked / height, 2) if height and stacked else 0, @@ -234,7 +252,7 @@ def import_assignments(cfg: Config, csv_path: Path) -> dict: if len(set(oids)) > 1: rejects.append( f"{name}: opening label {opening_ref!r} is ambiguous — " - "use the opening id or 'unit label'" + "use the opening id (see data/furniture.json)" ) continue oid = oids[0] @@ -244,7 +262,9 @@ def import_assignments(cfg: Config, csv_path: Path) -> dict: f"{openings[oid]['label']} in any orientation" ) continue - locations[key] = {"opening_id": oid, "note": ""} + # a re-imported plan must not erase a hand-entered note + note = (locations.get(key) or {}).get("note", "") + locations[key] = {"opening_id": oid, "note": note} assigned += 1 save_locations(cfg, locations) typer.echo(f"assigned {assigned} game(s); {len(rejects)} reject(s)") diff --git a/src/bggpipe/static/app.css b/src/bggpipe/static/app.css index 696fb9b..52fa7f8 100644 --- a/src/bggpipe/static/app.css +++ b/src/bggpipe/static/app.css @@ -543,6 +543,10 @@ select { #storedform select { flex: 1 1 auto; } /* -- shelves: units as grids of openings, sheet for one opening ------- */ +/* .card is flex for the review pages' photo-beside-body layout; a shelf + card is a vertical STACK — without this, unit headers share a row with + their grid and unshelved rows flow horizontally off the page */ +.card.stack { display: block; } .unit .unithead { display: flex; align-items: center; gap: .7rem; justify-content: space-between; flex-wrap: wrap; } .openings { display: grid; gap: .5rem; @@ -560,7 +564,7 @@ button.opening:hover, button.opening:focus-visible { .ocount { font-size: .82rem; color: var(--ink-soft); } .fillbar { height: .45rem; background: var(--board-edge); border-radius: 1rem; overflow: hidden; } -.fillbar > div { height: 100%; background: var(--ok, #4a7c4e); +.fillbar > div { height: 100%; background: var(--go, #2f8a45); border-radius: 1rem; } .fillbar > div.over { background: #b3372f; } .unshelved-row { display: flex; gap: .6rem; align-items: center; @@ -590,6 +594,13 @@ button.suggest:hover { border-color: var(--accent); } .sheetrow img { width: 2.2rem; height: 2.2rem; object-fit: cover; border-radius: .35rem; } .sheetrow .unassign { margin-left: auto; } +.addopeningform { margin-top: .6rem; } +#locform:not([hidden]) { display: flex; gap: .5rem; align-items: center; + flex-wrap: wrap; } +#locform select { max-width: 100%; flex: 1 1 14rem; } +#locform input { flex: 1 1 10rem; font: inherit; font-size: .85rem; + border: 2px solid var(--board-edge); border-radius: var(--radius); + padding: .25rem .45rem; } #sheetsearch { width: 100%; margin: .7rem 0 .35rem; font: inherit; font-size: 1rem; padding: .55rem .7rem; border: 2px solid var(--board-edge); border-radius: var(--radius); } diff --git a/src/bggpipe/templates/pages/library.html b/src/bggpipe/templates/pages/library.html index 2decf56..95e64cb 100644 --- a/src/bggpipe/templates/pages/library.html +++ b/src/bggpipe/templates/pages/library.html @@ -99,7 +99,9 @@ function render() { ].filter(Boolean).join(" ").toLowerCase().includes(q)); const where = document.getElementById("libwhere").value; if (where === "unshelved") - rows = rows.filter(g => !(g.location && (g.location.text || g.location.via))); + // same definition as the Shelves page: no opening and not riding a + // container — a note-only spot ("lent out") still counts as unshelved + rows = rows.filter(g => !(g.location && (g.location.unit || g.location.via))); else if (where) rows = rows.filter(g => g.location && g.location.unit === where); if (seats) rows = rows.filter(g => diff --git a/src/bggpipe/templates/pages/shelves.html b/src/bggpipe/templates/pages/shelves.html index b2026d5..4243eda 100644 --- a/src/bggpipe/templates/pages/shelves.html +++ b/src/bggpipe/templates/pages/shelves.html @@ -14,7 +14,7 @@ - +