diff --git a/booth/app.py b/booth/app.py
index 4184932..30b0e11 100644
--- a/booth/app.py
+++ b/booth/app.py
@@ -1971,7 +1971,14 @@ def create_app(
members = [r for r in ring if by_rel[r].group == item.group]
group = {"key": item.group, "k": members.index(f) + 1, "n": len(members)}
open_now = open_marks(marks)
- cring = _compare_ring(booth, items)
+ # The compare partner: the next ring item a compare can open,
+ # scanning forward (usually one step) rather than resolving the
+ # whole ring per render. Each rel is judged once; this item is
+ # re-judged first, so a race costs the control, never the page.
+ partner = None
+ if _in_booth(booth, f):
+ partner = next((r for r in (ring[(pos + k) % len(ring)] for k in range(1, len(ring) + 1))
+ if r == f or _in_booth(booth, r)), None)
return templates.TemplateResponse(
request, "view.html", {
**common,
@@ -1993,12 +2000,12 @@ def create_app(
"is_last": pos == len(ring) - 1,
"tray": [x for x in film if x["flagged"]],
"back_url": f"/b/{quote(name, safe='')}/#item-{item.url}",
- # R3 C2: this item against the NEXT in the compare ring
- # (itself in a ring of one), keyed by rel like every compare
- # URL. This item passed the containment check above, so it
- # is in the compare ring.
+ # R3 C2: this item against the NEXT comparable item in the
+ # ring (itself in a ring of one), keyed by rel like every
+ # compare URL; None, and no control, when it is not itself
+ # comparable any more (it vanished after the check above).
"compare_url": (f"/b/{quote(name, safe='')}/compare?a={quote(f, safe='/')}"
- f"&b={quote(cring[(cring.index(f) + 1) % len(cring)], safe='/')}"),
+ f"&b={quote(partner, safe='/')}") if partner else None,
})
# .md renders, .txt/.log show as text — viewable in-booth, no download
@@ -2039,16 +2046,15 @@ def create_app(
bug hunt, 3 of 4)."""
return [r for r in review_chain(items) if _in_booth(booth, r)]
- def _compare_side(booth: Path, ring: list[str], rel) -> str:
- """One side of a compare, or a 404 (R3 C1). A CONJUNCTION: the view
- route's resolve / containment / is_file check, AND membership of the
- review ring. The ring alone is not enough — `booth_items` follows
- symlinks, so a link pointing outside the booth is IN the ring and only
- containment refuses it. The view's check alone is not enough — it
- renders a doc, and compare takes media only."""
- if not _in_booth(booth, rel):
- raise HTTPException(status_code=404, detail="no such item")
- if rel not in ring:
+ def _compare_side(ring: list[str], rel) -> str:
+ """One side of a compare, or a 404 (R3 C1): membership of the COMPARE
+ ring, which is already the conjunction — the review ring (a doc is not
+ in it) filtered by the view route's containment (an outside symlink is
+ in the review ring and not in this one). Judged against the ONE ring
+ the route builds per request: resolving a rel twice lets a file that
+ vanishes between the two reach a `.index()` that raises, a 500
+ (booth-dev, after the merge)."""
+ if not isinstance(rel, str) or rel not in ring:
raise HTTPException(status_code=404, detail="no such item")
return rel
@@ -2073,9 +2079,9 @@ def create_app(
"""
booth = resolve_booth(name)
items = booth_items(booth)
- a = _compare_side(booth, review_chain(items), a)
- b = _compare_side(booth, review_chain(items), b)
- ring = _compare_ring(booth, items)
+ ring = _compare_ring(booth, items) # built ONCE; every rel judged once
+ a = _compare_side(ring, a)
+ b = _compare_side(ring, b)
# A look records both — below the 404s, so only a real pair counts.
record_view(booth)
record_seen(booth, a, items)
diff --git a/booth/templates/view.html b/booth/templates/view.html
index 3a12647..b4d94d5 100644
--- a/booth/templates/view.html
+++ b/booth/templates/view.html
@@ -14,7 +14,7 @@
{{ num(ordinal) }} {{ file }}
{# R3: this item against the next one in the ring, side by side (C). #}
- ⇆ compare
+ {% if compare_url %}⇆ compare{% endif %}
{% if kind == 'image' %}
{# A JS-only VIEWING convenience (INV-3): hidden until the script shows it,
and only ever rendered for a picture. With scripts off the image shows at
@@ -289,7 +289,7 @@
var b = document.getElementById('vflag-btn'); /* re-read: the rail may have been swapped */
if (b) { e.preventDefault(); b.click(); }
}
- else if (e.key === 'c' || e.key === 'C') { e.preventDefault(); window.location.href = COMPARE; }
+ else if ((e.key === 'c' || e.key === 'C') && COMPARE) { e.preventDefault(); window.location.href = COMPARE; }
else if (e.key === 'n' || e.key === 'N') {
var t = document.getElementById('vnote-text');
if (t) { e.preventDefault(); t.focus(); }
diff --git a/docs/contracts/r3_compare.contract.md b/docs/contracts/r3_compare.contract.md
index 6d3f60f..3a47f37 100644
--- a/docs/contracts/r3_compare.contract.md
+++ b/docs/contracts/r3_compare.contract.md
@@ -62,6 +62,13 @@ assumptions:
stays in the review ring). EVERY compare link is built from it: the strip,
the steps, the review's Compare control and the `back=compare` landing. So
no navigation offers a pair that 404s, and a step walks over such an item.
+- **Each rel is judged ONCE per request** (booth-dev, after the merge). The
+ route builds the compare ring once and judges both sides by membership of
+ it. Resolving a rel twice lets a file that vanishes between the two reach a
+ lookup that raises, a 500. The review re-judges its own item first and scans
+ forward for the next comparable one; when its item is no longer comparable,
+ the review renders WITHOUT a Compare control (and `C` does nothing), never a
+ 500.
- The response carries, per side: the rel, its quoted url, ordinal, kind,
caption, blurred, flagged and thumb. It also carries the compare ring as a
filmstrip in RING ORDER (the view route's `film`, one line in the route's docstring),
@@ -142,9 +149,10 @@ assumptions:
gap, never a border that comes out of one side's width. Two stages of
different sizes would draw the same picture at two scales in Fit.
- **At phone width (600px and below) a top bar that cannot hold its controls
- WRAPS** instead of scrolling the page sideways or crushing a control. This
- applies to the review's bar too, which gains the Compare control (only its
- glyph below 600px). The review's bar was already full: a fogged booth
+ WRAPS** instead of scrolling the page sideways or crushing a control. The
+ rule is on `.vbar`, so it applies to every bar of that class: compare's, the
+ review's (which gains the Compare control, only its glyph below 600px), and
+ doc.html's. The review's bar was already full: a fogged booth
overflowed it by 3px at 390px before r3.
- **Each stage is the r2c stage:** Fit fills (up or down, contain, never
cropped), or 1:1 at natural pixels with every pixel reachable. Drag pans a
diff --git a/tests/mutations/r3.toml b/tests/mutations/r3.toml
index b30c0c2..e38f1bf 100644
--- a/tests/mutations/r3.toml
+++ b/tests/mutations/r3.toml
@@ -19,11 +19,9 @@ label = "C1 the conjunction loses the ring (a doc or a sidecar opens as a side)"
file = "booth/app.py"
test = "tests/test_compare.py::test_a_bad_side_is_a_404"
old = '''
- if rel not in ring:
- raise HTTPException(status_code=404, detail="no such item")
- return rel'''
+ return [r for r in review_chain(items) if _in_booth(booth, r)]'''
new = '''
- return rel'''
+ return [it.rel for it in items if _in_booth(booth, it.rel)]'''
[[mutation]]
label = "C1 a missing side is FastAPI's 422 (no default)"
@@ -48,12 +46,12 @@ test = "tests/test_compare.py::test_a_look_records_both_seen"
old = '''
booth = resolve_booth(name)
items = booth_items(booth)
- a = _compare_side(booth, review_chain(items), a)'''
+ ring = _compare_ring(booth, items)'''
new = '''
booth = resolve_booth(name)
record_view(booth)
items = booth_items(booth)
- a = _compare_side(booth, review_chain(items), a)'''
+ ring = _compare_ring(booth, items)'''
[[mutation]]
label = "C6 compare does not carry data-booth (Reveal all and its restore bail)"
@@ -114,8 +112,8 @@ new = ''' side_a = side not in ("", "b")'''
label = "C2 the review's Compare does not wrap (the last item compares with itself)"
file = "booth/app.py"
test = "tests/test_compare.py::test_the_review_offers_compare_with_the_next_item"
-old = ''' f"&b={quote(cring[(cring.index(f) + 1) % len(cring)], safe='/')}"),'''
-new = ''' f"&b={quote(cring[min(cring.index(f) + 1, len(cring) - 1)], safe='/')}"),'''
+old = '''(ring[(pos + k) % len(ring)] for k in range(1, len(ring) + 1))'''
+new = '''(ring[min(pos + k, len(ring) - 1)] for k in range(1, len(ring) + 1))'''
# ---- C5: the regions and the JS-off flag landing
@@ -543,6 +541,32 @@ old = '''
new = '''
return p && !/^(side|link)(=|$)/.test(p);'''
+# ---- after the merge: booth-dev's race note (01M3AT7GKCPATJD5YW0PR3SRPT)
+
+[[mutation]]
+label = "C1 a side is judged twice (the ring rebuilt per side): a side that vanishes between is a 500"
+file = "booth/app.py"
+test = "tests/test_compare.py::test_a_side_that_vanishes_mid_request_never_500s"
+old = '''
+ ring = _compare_ring(booth, items) # built ONCE; every rel judged once
+ a = _compare_side(ring, a)
+ b = _compare_side(ring, b)'''
+new = '''
+ a = _compare_side(_compare_ring(booth, items), a)
+ b = _compare_side(_compare_ring(booth, items), b)
+ ring = _compare_ring(booth, items)'''
+
+[[mutation]]
+label = "C2 the review offers Compare for an item that vanished after its own check"
+file = "booth/app.py"
+test = "tests/test_compare.py::test_the_review_hides_compare_when_its_item_vanishes_mid_request"
+old = '''
+ if _in_booth(booth, f):
+ partner'''
+new = '''
+ if True:
+ partner'''
+
# Refuted, not rowed (bug hunt): "a right-click mid-drag ends the pan" — a
# second button pressed and released during a drag arrives as chorded
# `pointermove` events, never a `pointerup` (measured 3/3 in the test browser,
diff --git a/tests/test_compare.py b/tests/test_compare.py
index d117ed1..fa6b883 100644
--- a/tests/test_compare.py
+++ b/tests/test_compare.py
@@ -165,6 +165,54 @@ def test_a_planted_fifo_marker_cannot_hang_a_look(tmp_path):
assert got == [200], "a planted FIFO held the look open"
+def _vanish_after_scan(monkeypatch, name: str, grace: int) -> None:
+ """Make `name` stop being a file partway through a request: once
+ booth_items has scanned the booth, the first `grace` is_file checks of it
+ still pass and every later one fails — a file deleted or relinked outside
+ the booth mid-request, between two resolves of the same rel."""
+ import pathlib as _pl
+ import booth.app as app_mod
+ state = {"armed": False, "calls": 0}
+ real_items, real_is_file = app_mod.booth_items, _pl.Path.is_file
+
+ def items(booth):
+ out = real_items(booth)
+ state["armed"] = True
+ return out
+
+ def is_file(self):
+ if state["armed"] and self.name == name:
+ state["calls"] += 1
+ if state["calls"] > grace:
+ return False
+ return real_is_file(self)
+
+ monkeypatch.setattr(app_mod, "booth_items", items)
+ monkeypatch.setattr(_pl.Path, "is_file", is_file)
+
+
+def test_a_side_that_vanishes_mid_request_never_500s(tmp_path, monkeypatch):
+ """booth-dev's race: each rel must be judged ONCE per request. A side that
+ passes its check and then vanishes before a second resolve must not reach
+ a `.index()` that raises — a damaged file costs its own tile, never the
+ page."""
+ _booth(tmp_path, "g", {"p.png": PNG, "q.png": PNG, "r.png": PNG})
+ _vanish_after_scan(monkeypatch, "p.png", grace=1)
+ r = _client(tmp_path).get("/b/g/compare?a=p.png&b=q.png")
+ assert r.status_code in (200, 404), r.status_code
+
+
+def test_the_review_hides_compare_when_its_item_vanishes_mid_request(tmp_path, monkeypatch):
+ """The review checked its item, then the item vanished before the compare
+ ring was built: the page still renders, without a Compare control (a
+ compare of it would 404) — never a 500."""
+ _booth(tmp_path, "g", {"p.png": PNG, "q.png": PNG})
+ _vanish_after_scan(monkeypatch, "p.png", grace=0)
+ r = _client(tmp_path).get("/b/g/view?f=p.png")
+ assert r.status_code == 200, r.status_code
+ assert 'class="vbtn vcompare"' not in r.text
+
+
def test_a_look_records_both_seen(tmp_path):
"""A compare GET is a look at both sides; a 404 records nothing."""
import json