Credibility pass 2: the copy-then-drift tells, excised
Two blind reviewers swept the 33 commits since 10f65d8 for signs of
machine generation. Verdict: production code and copy largely clean;
the tells clustered in duplication and tests.
JS: the six-times-pasted change-detection loop (three pages honoring a
LAST-after-render invariant, three violating it) becomes one
changeGate() factory in app.js; the reshoot ticket renderer and
dismiss wiring, duplicated across photos/photo pages, become
ticketCard()/wireDismiss(); review.html's hand-rolled fetch/post
collapse onto fetchJSON/apiPost keeping only its unique
saved-but-render-failed path; dead lastGood deleted; page-state naming
unified to CAPS (ACTIVE, RUNNING); a dead defensive rowix branch gone.
CSS: header no longer claims "two pages"; --focus derives from
--accent; five state tints become tokens (the header's tokens-for-roles
promise, kept); component button rules drop declarations the global
rule supplies; duplicate color declarations trimmed.
Python: dead seen_per_title vestige removed from resolve; redundant
ternary arm in the catalog builder collapsed; csv import hoisted; twin
VetoBody/SplitBody merged into RowRef; warn-once idiom deduplicated
into a closure; a stray "a bare arrays" typo.
Tests: the one assertion that could never fail (aria-current check
with an always-true fallback) replaced by a strict per-page check
across all seven pages; the traversal test asserts escape
unconditionally; stale "both pages" names updated; nine redundant
function-local imports hoisted to their module tops.
Docs: aria role="status" set once in the shell instead of per call;
joblog gets role="log"; README's --lan paragraph becomes a proper
"From your phone" quickstart subsection with the command visible, and
the seven-page list stops restating the screenshot captions; Help's
re-extract claim matches actual behavior.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016jXZFSTZQKzAC8fqpWSz9g
This commit is contained in:
co-authored by
Claude Fable 5
parent
7cfc3c5b7d
commit
0cdbf74a02
@@ -4,6 +4,8 @@ from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from bggpipe.config import Config, load_config
|
||||
|
||||
|
||||
@@ -40,8 +42,6 @@ def test_username_comes_from_env_only(tmp_path, monkeypatch):
|
||||
|
||||
def test_unknown_toml_keys_warn(tmp_path, monkeypatch):
|
||||
# a typo'd knob must not silently fall back to defaults
|
||||
import pytest
|
||||
|
||||
monkeypatch.delenv("BGG_USERNAME", raising=False)
|
||||
p = tmp_path / "config.toml"
|
||||
p.write_text('photo_dir = "oops"\n')
|
||||
@@ -50,7 +50,5 @@ def test_unknown_toml_keys_warn(tmp_path, monkeypatch):
|
||||
|
||||
|
||||
def test_explicit_missing_config_errors_instead_of_silent_defaults(tmp_path):
|
||||
import pytest
|
||||
|
||||
with pytest.raises(FileNotFoundError, match="does not exist"):
|
||||
load_config(tmp_path / "nope.toml")
|
||||
|
||||
@@ -486,9 +486,6 @@ def test_corrupt_raw_cache_is_reextracted_not_skipped(tmp_path):
|
||||
|
||||
|
||||
def test_systemic_failures_abort_and_exit_nonzero(tmp_path):
|
||||
import pytest
|
||||
import typer as _typer
|
||||
|
||||
calls = []
|
||||
|
||||
def broken_vision(image_b64, media_type):
|
||||
@@ -499,6 +496,6 @@ def test_systemic_failures_abort_and_exit_nonzero(tmp_path):
|
||||
cfg.photos_dir.mkdir()
|
||||
for i in range(6):
|
||||
_write_image(cfg.photos_dir / f"p{i}.jpg")
|
||||
with pytest.raises(_typer.Exit):
|
||||
with pytest.raises(typer.Exit):
|
||||
run_extract(cfg, vision=broken_vision)
|
||||
assert len(calls) == 3 # aborted after 3 identical failures
|
||||
|
||||
@@ -505,8 +505,6 @@ def test_split_row_makes_per_photo_vetoed_copies(tmp_path):
|
||||
|
||||
|
||||
def test_split_copies_survive_resolve_rerun(tmp_path):
|
||||
import json as _json
|
||||
|
||||
from bggpipe.resolve import read_matches, run_resolve
|
||||
|
||||
cfg = _setup(
|
||||
@@ -521,7 +519,7 @@ def test_split_copies_survive_resolve_rerun(tmp_path):
|
||||
],
|
||||
)
|
||||
(cfg.data_dir / "titles.json").write_text(
|
||||
_json.dumps(
|
||||
json.dumps(
|
||||
[{"title_raw": "Wiz-War", "source_photos": ["a.jpg", "b.jpg", "c.jpg"]}]
|
||||
)
|
||||
)
|
||||
|
||||
+17
-26
@@ -3,6 +3,7 @@ stage functions are injected so nothing slow or networked ever runs."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import threading
|
||||
import time
|
||||
|
||||
@@ -174,9 +175,11 @@ def test_photo_upload_rejects_non_photos_and_path_tricks(tmp_path):
|
||||
"/api/photos",
|
||||
files={"files": ("../../escape.jpg", b"x", "image/jpeg")},
|
||||
)
|
||||
if res.status_code == 200: # client may strip the path; the name must be bare
|
||||
# whether the server accepts a stripped bare name or rejects outright,
|
||||
# nothing may land outside photos_dir
|
||||
assert not (tmp_path / "escape.jpg").exists()
|
||||
if res.status_code == 200:
|
||||
assert (cfg.photos_dir / "escape.jpg").exists()
|
||||
assert not (tmp_path / "escape.jpg").exists()
|
||||
|
||||
|
||||
# -- pages --------------------------------------------------------------
|
||||
@@ -191,7 +194,7 @@ def test_dashboard_and_review_pages_serve(tmp_path):
|
||||
# -- design system + navigation -----------------------------------------
|
||||
|
||||
|
||||
def test_stylesheet_is_served_and_linked_by_both_pages(tmp_path):
|
||||
def test_stylesheet_is_served_and_linked_by_every_page(tmp_path):
|
||||
web = _app(_cfg(tmp_path))
|
||||
css = web.get("/static/app.css")
|
||||
assert css.status_code == 200
|
||||
@@ -201,15 +204,12 @@ def test_stylesheet_is_served_and_linked_by_both_pages(tmp_path):
|
||||
assert 'href="/static/app.css"' in web.get(path).text
|
||||
|
||||
|
||||
def test_both_pages_carry_navigation_and_skip_link(tmp_path):
|
||||
def test_every_page_marks_itself_current_in_the_nav(tmp_path):
|
||||
web = _app(_cfg(tmp_path))
|
||||
for path, current in (("/", 'href="/"'), ("/review", 'href="/review"')):
|
||||
for path in ("/", "/photos", "/titles", "/review", "/queue", "/library", "/help"):
|
||||
html = web.get(path).text
|
||||
assert 'aria-label="Primary"' in html
|
||||
assert f'<a {current} aria-current="page"' in html.replace("\n", " ") or (
|
||||
current in html and 'aria-current="page"' in html
|
||||
)
|
||||
assert 'class="skip"' in html
|
||||
assert f'href="{path}" aria-current="page"' in html, path
|
||||
assert 'class="skip"' in html, path
|
||||
|
||||
|
||||
def test_activity_region_announces_politely(tmp_path):
|
||||
@@ -240,14 +240,13 @@ def test_every_page_serves_with_shared_shell(tmp_path):
|
||||
|
||||
|
||||
def test_photos_list_reports_extraction_state(tmp_path):
|
||||
import json as _json
|
||||
|
||||
cfg = _cfg(tmp_path)
|
||||
(cfg.photos_dir / "done.jpg").write_bytes(b"x")
|
||||
(cfg.photos_dir / "fresh.jpg").write_bytes(b"x")
|
||||
cfg.extract_raw_dir.mkdir(parents=True)
|
||||
(cfg.extract_raw_dir / "done.jpg.json").write_text(
|
||||
_json.dumps({"titles": [{"title_raw": "Catan"}], "unidentified": [{}, {}]})
|
||||
json.dumps({"titles": [{"title_raw": "Catan"}], "unidentified": [{}, {}]})
|
||||
)
|
||||
listing = {p["name"]: p for p in _app(cfg).get("/api/photos-list").json()}
|
||||
assert listing["done.jpg"] == {
|
||||
@@ -268,13 +267,12 @@ def test_queue_endpoint_serves_all_three_ledgers(tmp_path):
|
||||
|
||||
|
||||
def test_library_serves_games_sorted_or_empty(tmp_path):
|
||||
import json as _json
|
||||
|
||||
cfg = _cfg(tmp_path)
|
||||
web = _app(cfg)
|
||||
assert web.get("/api/library").json() == []
|
||||
cfg.games_path.write_text(
|
||||
_json.dumps(
|
||||
json.dumps(
|
||||
{
|
||||
"13": {"name": "Catan", "year": 1995},
|
||||
"266192": {"name": "Wingspan", "year": 2019},
|
||||
@@ -287,23 +285,19 @@ def test_library_serves_games_sorted_or_empty(tmp_path):
|
||||
|
||||
|
||||
def test_pipeline_reports_reshoot_count(tmp_path):
|
||||
import json as _json
|
||||
|
||||
cfg = _cfg(tmp_path)
|
||||
cfg.unidentified_path.write_text(
|
||||
_json.dumps({"a.jpg": [{"location": "top shelf"}]})
|
||||
)
|
||||
cfg.unidentified_path.write_text(json.dumps({"a.jpg": [{"location": "top shelf"}]}))
|
||||
assert _app(cfg).get("/api/pipeline").json()["reshoot"] == 1
|
||||
|
||||
|
||||
def test_photos_list_tolerates_bare_array_raw_cache(tmp_path):
|
||||
import json as _json
|
||||
|
||||
cfg = _cfg(tmp_path)
|
||||
(cfg.photos_dir / "old.jpg").write_bytes(b"x")
|
||||
cfg.extract_raw_dir.mkdir(parents=True)
|
||||
(cfg.extract_raw_dir / "old.jpg.json").write_text(
|
||||
_json.dumps([{"title_raw": "Catan"}, {"title_raw": "Risk"}])
|
||||
json.dumps([{"title_raw": "Catan"}, {"title_raw": "Risk"}])
|
||||
)
|
||||
(item,) = _app(cfg).get("/api/photos-list").json()
|
||||
assert item["extracted"] is True and item["titles"] == 2
|
||||
@@ -322,7 +316,6 @@ def test_non_photo_files_are_invisible(tmp_path):
|
||||
|
||||
|
||||
def test_running_snapshot_shows_partial_line_then_finishes():
|
||||
import typer as _typer
|
||||
|
||||
runner = JobRunner()
|
||||
release = threading.Event()
|
||||
@@ -330,7 +323,7 @@ def test_running_snapshot_shows_partial_line_then_finishes():
|
||||
def stage():
|
||||
print("progress: 40%", end="", flush=True) # no newline yet
|
||||
release.wait()
|
||||
_typer.echo(" done")
|
||||
typer.echo(" done")
|
||||
|
||||
runner.start("extract", stage)
|
||||
for _ in range(100):
|
||||
@@ -346,7 +339,6 @@ def test_running_snapshot_shows_partial_line_then_finishes():
|
||||
|
||||
|
||||
def test_log_serves_last_200_lines_and_buffer_is_bounded():
|
||||
import typer as _typer
|
||||
|
||||
from bggpipe.jobs import MAX_LOG_LINES
|
||||
|
||||
@@ -354,7 +346,7 @@ def test_log_serves_last_200_lines_and_buffer_is_bounded():
|
||||
|
||||
def stage():
|
||||
for i in range(MAX_LOG_LINES + 300):
|
||||
_typer.echo(f"line {i}")
|
||||
typer.echo(f"line {i}")
|
||||
|
||||
runner.start("extract", stage)
|
||||
runner.wait()
|
||||
@@ -364,9 +356,8 @@ def test_log_serves_last_200_lines_and_buffer_is_bounded():
|
||||
|
||||
|
||||
def test_zero_exit_codes_count_as_done():
|
||||
import typer as _typer
|
||||
|
||||
for exc in (_typer.Exit(), SystemExit(0)):
|
||||
for exc in (typer.Exit(), SystemExit(0)):
|
||||
runner = JobRunner()
|
||||
runner.start("diff", lambda exc=exc: (_ for _ in ()).throw(exc))
|
||||
runner.wait()
|
||||
|
||||
@@ -3,6 +3,7 @@ no live BGG (the injected client 401s on any cache miss)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
import json
|
||||
|
||||
import httpx
|
||||
@@ -408,7 +409,6 @@ def test_session_warnings_surface_in_state(tmp_path):
|
||||
def test_duplicate_rows_are_individually_decidable_via_row_ix(tmp_path):
|
||||
# two editions of one game in one photo: byte-identical rows. The
|
||||
# ordinal must land each decision on its own row.
|
||||
from bggpipe.resolve import read_matches as read_m
|
||||
from bggpipe.resolve import write_matches as write_m
|
||||
|
||||
cfg = make_cfg(tmp_path)
|
||||
@@ -430,7 +430,7 @@ def test_duplicate_rows_are_individually_decidable_via_row_ix(tmp_path):
|
||||
"action": "reject",
|
||||
},
|
||||
)
|
||||
rows = read_m(cfg.matches_path)
|
||||
rows = read_matches(cfg.matches_path)
|
||||
assert [r["match_status"] for r in rows] == ["unmatched", "rejected"]
|
||||
|
||||
|
||||
@@ -605,8 +605,6 @@ def test_edit_preserves_vetoed_rows_and_renames_them(tmp_path):
|
||||
|
||||
|
||||
def test_drop_rows_photo_narrowing_spares_other_copy(tmp_path):
|
||||
import io as _io
|
||||
|
||||
from rich.console import Console
|
||||
|
||||
from bggpipe.review import ReviewSession
|
||||
@@ -621,7 +619,7 @@ def test_drop_rows_photo_narrowing_spares_other_copy(tmp_path):
|
||||
)
|
||||
session = ReviewSession(
|
||||
cfg,
|
||||
console=Console(file=_io.StringIO()),
|
||||
console=Console(file=io.StringIO()),
|
||||
input_fn=lambda prompt: "",
|
||||
client=unauthorized_client(tmp_path),
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user