perf(images): read positive counts only on a plain listing's first page

py/api/images.py

@@ -559,6 +559,11 @@ def _workspace_filters(query: Mapping) -> dict:
     }
 
 
+# A plain listing's cursor page leaves positive_count out (see
+# _select_workspace_page); None instead means the count could not be read.
+_COUNT_OMITTED = object()
+
+
 def _filtered_page(
     entries: list, start: int, size: int, filters: dict, links: dict | None
 ) -> tuple[int, list, int | None]:
@@ -978,17 +983,18 @@ class ImageRoutesMixin:
             )
         )
         images = await self._run_in_executor(self._format_workspace_page, page)
-        return web.json_response(
-            {
-                "success": True,
-                "images": images,
-                "total": total,
-                "positive_count": positive_count,
-                "limit": limit,
-                "has_more": has_more,
-                "next_cursor": next_cursor,
-            }
-        )
+        result = {
+            "success": True,
+            "images": images,
+            "total": total,
+            "positive_count": positive_count,
+            "limit": limit,
+            "has_more": has_more,
+            "next_cursor": next_cursor,
+        }
+        if positive_count is _COUNT_OMITTED:
+            del result["positive_count"]
+        return web.json_response(result)
 
     def _workspace_listed(self, index: int, root: Path, files: list) -> list:
         """The _Listed values of one root's scan, rebuilt only where it changed.
@@ -1069,24 +1075,36 @@ class ImageRoutesMixin:
 
         One SELECT (get_image_link_matches) reads every linked positive of the
         listing: the filters, the rating sort and the positive count share it.
-        Without a filter that needs it, a failed read leaves the listing
-        usable and the positive count unknown (None).
+        A plain listing (no ``q``, category, tags or rating sort) reads it only
+        for its first page; its cursor pages read no links and the returned
+        count is _COUNT_OMITTED, so the response leaves ``positive_count`` out
+        and the frontend keeps the first page's value. On that plain first
+        page a failed read leaves the listing usable and the count unknown
+        (None).
         """
         order, category, tags = filters["order"], filters["category"], filters["tags"]
-        needed = bool(filters["query"]) or category is not None or bool(tags)
-        try:
-            links = self.db.get_image_link_matches(
-                filters["text"],
-                category,
-                tags,
-                every_link=True,
-                wanted=self._workspace_wanted(listings),
-            )
-        except Exception:
-            if needed or order == "rating":
-                raise
-            self.logger.warning("Workspace positive count could not be read")
-            links = None
+        needed = (
+            bool(filters["query"])
+            or category is not None
+            or bool(tags)
+            or order == "rating"
+        )
+        links = None
+        if needed or after is None:
+            try:
+                links = self.db.get_image_link_matches(
+                    filters["text"],
+                    category,
+                    tags,
+                    every_link=True,
+                    wanted=self._workspace_wanted(listings),
+                )
+            except Exception:
+                if needed:
+                    raise
+                self.logger.warning(
+                    "Workspace positive count could not be read", exc_info=True
+                )
         if order == "rating":
 
             def rank(entry):
@@ -1118,6 +1136,8 @@ class ImageRoutesMixin:
         total, page, positive_count = _filtered_page(
             entries, start, limit + 1, filters, links
         )
+        if not needed and after is not None:
+            positive_count = _COUNT_OMITTED
         has_more = len(page) > limit
         page = page[:limit]
         next_cursor = _workspace_cursor(key(page[-1]), scope) if has_more else None

tests/test_api_workspace_images.py

@@ -854,14 +854,28 @@ class TestWorkspaceImages(ImageAPITestCase):
                 self.assertEqual(response.status, 200, data)
                 self.assertEqual(data["total"], total)
                 self.assertEqual(data["positive_count"], positives)
-        # The count covers the whole matching set, on every cursor page.
-        response, first = await self.get(limit=2, q="ZEBRA")
-        self.assertEqual(first["positive_count"], 4)
-        response, second = await self.get(
-            limit=2, q="ZEBRA", cursor=first["next_cursor"]
-        )
-        self.assertEqual(response.status, 200, second)
-        self.assertEqual(second["positive_count"], 4)
+        # The count covers the whole matching set. A filter or the rating
+        # sort reads the links on every page, so its cursor pages carry it too.
+        for params in ({"q": "ZEBRA"}, {"sort": "rating"}):
+            with self.subTest(cursor=params):
+                response, first = await self.get(limit=2, **params)
+                response, second = await self.get(
+                    limit=2, cursor=first["next_cursor"], **params
+                )
+                self.assertEqual(response.status, 200, second)
+                self.assertEqual(second["positive_count"], first["positive_count"])
+        # A plain cursor page skips the read and omits the key (absent, not
+        # null): the frontend keeps the first page's value.
+        for params in ({}, {"sort": "oldest"}, {"sort": "name"}):
+            with self.subTest(plain_cursor=params):
+                response, first = await self.get(limit=2, **params)
+                self.assertEqual(first["positive_count"], 6)
+                response, second = await self.get(
+                    limit=2, cursor=first["next_cursor"], **params
+                )
+                self.assertEqual(response.status, 200, second)
+                self.assertNotIn("positive_count", second)
+                self.assertEqual(second["total"], 10)
         # One SELECT counts the positives; the page's link summary is the other.
         statements = []
         original = db.model.get_connection
@@ -880,6 +894,18 @@ class TestWorkspaceImages(ImageAPITestCase):
             if query.lstrip().upper().startswith(("SELECT", "WITH"))
         ]
         self.assertEqual(len(selects), 2, selects)
+        _, first = await self.get(limit=2)
+        statements.clear()
+        with patch.object(db.model, "get_connection", side_effect=trace_connection):
+            response, data = await self.get(limit=2, cursor=first["next_cursor"])
+        self.assertEqual(response.status, 200)
+        selects = [
+            query
+            for query in statements
+            if query.lstrip().upper().startswith(("SELECT", "WITH"))
+        ]
+        self.assertEqual(len(selects), 1, selects)  # the page's link summary only
+        self.assertNotIn("gi.file_path, p.rating", selects[0])
         # Without filters the listing outlives a database failure; the count
         # is then unknown rather than wrong.
         with patch.object(
@@ -1167,6 +1193,9 @@ class TestWorkspaceImages(ImageAPITestCase):
         return listed
 
     _BUDGET_REQUESTS = (
+        # First, so the timed loop also measures a cursor page of each.
+        {},
+        {"sort": "oldest"},
         {"q": "lantern"},
         {"q": "img_0123"},
         {"q": "Title 1"},
@@ -1260,12 +1289,17 @@ class TestWorkspaceImages(ImageAPITestCase):
                     await self.get(sort=order, limit=10)  # builds a lazy order
                     _, first = await self.get(sort=order, limit=10)
                     keyed.clear()
+                    selects()
                     response, page = await self.get(
                         sort=order, limit=10, cursor=first["next_cursor"]
                     )
                     self.assertEqual(response.status, 200)
                     self.assertEqual(len(page["images"]), 10)
                     self.assertLessEqual(len(keyed), bound)
+                    # A plain cursor page reads no links; rating needs them.
+                    plain = order != "rating"
+                    self.assertEqual(len(selects()), 0 if plain else 1)
+                    self.assertEqual("positive_count" in page, not plain)
             key = ((0, str(self.output_dir)),)
             # A TTL rescan that found the same files rebuilds nothing.
             built.clear()
@@ -1319,7 +1353,7 @@ class TestWorkspaceImages(ImageAPITestCase):
         listed = await asyncio.get_running_loop().run_in_executor(
             None, self._budget_fixture, 50_000, 20_000
         )
-        rounds = []
+        rounds, plain = [], {"first": [], "cursor": []}
         with (
             patch.object(
                 self.api, "_get_gallery_files", AsyncMock(return_value=listed)
@@ -1335,7 +1369,8 @@ class TestWorkspaceImages(ImageAPITestCase):
                 timings = []
                 for index in range(20):
                     requests = self._BUDGET_REQUESTS
-                    params = dict(requests[index % len(requests)], limit=100)
+                    shape = requests[index % len(requests)]
+                    params = dict(shape, limit=100)
                     if index >= len(requests):
                         _, first = await self.get(**params)
                         if first["next_cursor"]:
@@ -1343,15 +1378,28 @@ class TestWorkspaceImages(ImageAPITestCase):
                     started = time.perf_counter()
                     response, data = await self.get(**params)
                     timings.append(time.perf_counter() - started)
+                    if shape.get("sort", "newest") in ("newest", "oldest") and not (
+                        set(shape) - {"sort"}
+                    ):
+                        # Plain first pages read the links for the count;
+                        # plain cursor pages read none.
+                        plain["cursor" if "cursor" in params else "first"].append(
+                            timings[-1]
+                        )
                     self.assertEqual(response.status, 200, (params, data))
                 timings.sort()
                 rounds.append(timings)
         p95s = [timings[math.ceil(0.95 * len(timings)) - 1] for timings in rounds]
         best = min(p95s)
+        plain_ms = {
+            kind: sorted(round(t * 1000, 1) for t in found)
+            for kind, found in plain.items()
+        }
         print(
             "\nworkspace filter budget: best p95="
             f"{best * 1000:.1f} ms, rounds="
-            f"{[round(p * 1000, 1) for p in p95s]} ms, cold={cold * 1000:.1f} ms",
+            f"{[round(p * 1000, 1) for p in p95s]} ms, cold={cold * 1000:.1f} ms, "
+            f"plain listing ms={plain_ms}",
             file=sys.stderr,
         )
         self.assertLess(