Reviews: --sort reaches the store, and page 2 continues from page 1

.github/workflows/canary.yml

@@ -147,7 +147,7 @@ jobs:
           # real people's names and profile ids into a public artefact.
           python3 playwright_scraper.py \
             --mode reviews --app-id com.whatsapp --pages 2 \
-            --reviews-per-call 100 --no-authors \
+            --reviews-per-call 100 --sort newest --no-authors \
             --format json --out canary_reviews
           echo "exit_code=$?" >> "$GITHUB_OUTPUT"
 
@@ -168,10 +168,21 @@ jobs:
           ok &= require("exit code is 0", rc == 0, f"got {rc}")
           ok &= require("status is complete", meta["status"] == "complete",
                         meta["status"])
-          # Page 1 is embedded in the app page and page 2 comes from
-          # batchexecute, so clearing 100 rows proves BOTH routes worked.
-          ok &= require("both review routes produced rows", len(rows) >= 100,
-                        str(len(rows)))
+          # Both pages come from batchexecute, page 2 with page 1's token, so
+          # each carries close to the 100 asked for. The old floor was
+          # `>= 100` over the whole run and passed by accident: 20 embedded
+          # reviews plus the 80 a TOKENLESS page 2 had left after dedupe.
+          # A per-page floor is what fails when page 2 restarts the list.
+          per_page = [sum(1 for r in rows if r["page"] == n) for n in (1, 2)]
+          ok &= require("each page carries a full page of reviews",
+                        all(n >= 95 for n in per_page), str(per_page))
+          # --sort newest must come back newest first ACROSS the page
+          # boundary. Counting rows could not see that it did not.
+          dates = [r["posted_at"] for r in rows]
+          ok &= require("newest first, end to end",
+                        all(dates[i] >= dates[i + 1]
+                            for i in range(len(dates) - 1)),
+                        str(dates[:4]))
           ok &= require("review ids are unique",
                         len({r["review_id"] for r in rows}) == len(rows))
           ok &= require("scores are real stars",

CHANGELOG.md

@@ -11,6 +11,40 @@ discovered from a bill or a diff.
 
 ## [Unreleased]
 
+> **Correction to 0.1.0 and 0.2.0: `--sort newest` did not sort, and the
+> README's account of the store's orderings was wrong.** Both releases sent
+> the sort key in the wrong slot of the request. The store then answered
+> "newest" in its relevance order — dates like 08-28, 08-31, 09-18, 07-26 —
+> and answered "relevant" and "rating" with nothing at all, which the README
+> reported as "the store's other two keys return an empty payload". That was
+> a measurement of this repository's own request, not of the store: in the
+> request shape the store's front end uses, all three orderings work.
+> Reviews runs will now come back in a different order, and `--sort` takes
+> two more values.
+
+### Fixed
+
+- **`--sort` reaches the store.** The key now goes in the second slot of the
+  sort block (`[2, <sort>, [count, null, token], null, []]`). `newest` is
+  newest first and stays so across pages; `relevant` and `rating` are
+  offered. An unknown value is refused rather than silently replaced.
+- **Page 2 of a reviews run no longer restarts the list.** Page 1 used to be
+  the handful of reviews embedded in the app page, which carry no
+  continuation token, so page 2 went out without one, came back as the start
+  of the list, and was mostly removed as duplicates — 20 / 1 / 19 rows over
+  three pages in one reported run, and `end_of_listing` after a single page
+  in another. Every page now comes from the reviews endpoint, each asked for
+  with the previous page's token. That costs one request more than before.
+- `--url <app page> --mode reviews` runs a reviews run. It used to be refused
+  as a mode conflict, contradicting `--help`.
+- With a proxy pool, a reviews run logs that it keeps one exit for the run
+  (the continuation token belongs to the session) instead of claiming the
+  per-page rotation it never applies.
+- The canary's reviews job asserts a full page of rows on EACH page and
+  newest-first order across the run. Its previous floor of 100 rows over two
+  pages passed by accident: 20 embedded reviews plus the 80 a tokenless page
+  2 left after dedupe.
+
 ### Changed
 
 - CI only: the canary now runs weekly (Mondays 06:17 UTC) instead of daily, and on a manual dispatch.

README.md

@@ -168,16 +168,28 @@ capped there. Measured 2026-09-22 on one app: **100 → 100, 200 → 200,
 continuation token. No ceiling was found. The default here is 200;
 `--reviews-per-call` raises it.
 
-Page 1 costs no extra request at all — the app page already embeds it.
-
-Only `--sort newest` is implemented. The store's front end also offers "most
-relevant" and "rating", and in this request shape both return an empty
-payload: measured on two apps, sort key 1 → 0 reviews, key 2 → 40 and a
-token, key 3 → 0. That is a TODO about this request shape, not a limit of the
-store, and the flag offers only what was measured to work. There is likewise
-no score filter: the payload has a slot for one and setting it changes
-nothing (asking for score 1 and for score 5 each returned forty reviews
-spanning all five).
+Every page comes from that endpoint, page 2 onward asked for with the
+previous page's continuation token. The app page is still fetched first —
+to confirm the app exists in that market, which Play answers with a clean
+404 when it does not — but its own embedded handful of reviews is not used
+as page 1: those come in relevance order whatever `--sort` says and carry no
+token to continue from.
+
+`--sort` takes the store's three orderings:
+
+| `--sort` | what comes back |
+|---|---|
+| `newest` (default) | posted date descending, kept across pages |
+| `relevant` | the store's own ranking |
+| `rating` | grouped by score |
+
+Measured 2026-10-22 on `com.whatsapp`: `newest` returned two pages of 40
+with no review shared between them and `posted_at` non-increasing across the
+page boundary; `relevant` and `rating` each returned their own ordering.
+
+This repository does not implement a score filter. The store's front end has
+one; the request forms tried so far either changed nothing or returned no
+rows, so no flag is offered until one is measured to filter.
 
 **Reviews are about identifiable people.** Play publishes a reviewer's
 display name, avatar and a stable 21-digit profile id beside their words.

captures/README.md

@@ -5,6 +5,10 @@ address (netcup, Nuremberg) with plain `curl` and no cookies. Every number in
 the README and in `product_parser.py`'s comments can be re-measured from
 these.
 
+The two `reviews_*.txt` files were re-taken on 2026-10-22, after the request
+`make_fixtures.py` sends was corrected to put the sort key where the store
+reads it; the earlier pair was in relevance order.
+
 | file | route | what it is for |
 |---|---|---|
 | `cat_game_us.html` | `/store/apps/category/GAME` | the grid, and seven named shelves |
@@ -19,8 +23,8 @@ these.
 | `apps_home_us.html` | `/store/apps` | the store front, with the chart shelves |
 | `cluster_us.html` | `/store/apps/collection/cluster?gsr=…` | an expanded shelf — what `--pages` follows |
 | `notfound.html` | a package id that does not exist | HTTP 404, and the control for the asset-count signal |
-| `reviews_p1.txt` | `batchexecute` RPC `UsvDTd` | 40 reviews and a continuation token |
-| `reviews_p2.txt` | the same, with that token | 40 more, sharing none of page 1's ids |
+| `reviews_p1.txt` | `batchexecute` RPC `UsvDTd`, `--sort newest` | 40 reviews, newest first, and a continuation token |
+| `reviews_p2.txt` | the same, with that token | 40 more, sharing none of page 1's ids and continuing the date order |
 
 ## What is NOT verbatim
 

engine_core.py

@@ -126,9 +126,9 @@ def build_parser() -> argparse.ArgumentParser:
                          f"to 2000)")
     ap.add_argument("--sort", default=parser.DEFAULT_REVIEW_SORT,
                     choices=sorted(parser.REVIEW_SORTS),
-                    help="review ordering. Only 'newest' is implemented: the "
-                         "store's other two keys return an empty payload in "
-                         "this request shape.")
+                    help="review ordering: newest (posted_at descending, "
+                         "kept across pages), relevant (the store's own "
+                         "ranking) or rating. Default newest.")
     ap.add_argument("--no-authors", action="store_true",
                     help="drop reviewer name, profile id and avatar from "
                          "review rows")
@@ -192,6 +192,12 @@ def resolve_target(args: argparse.Namespace) -> Tuple[Optional[str], Optional[st
         if reason:
             return None, None, reason
         mode = parser.mode_for_url(args.url)
+        # An app page is the address of BOTH app-level modes: `--mode app`
+        # reads the page, `--mode reviews` pages that app's reviews. Refusing
+        # the second contradicted `--help`, which says the route decides the
+        # mode unless --mode says otherwise.
+        if mode == "app" and args.mode == "reviews":
+            return "reviews", parser.with_market(args.url, args.hl, args.gl), None
         if args.mode and args.mode != "listing" and mode != args.mode:
             # An explicit --mode that disagrees with the URL is a mistake
             # worth naming rather than silently resolving one way.
@@ -578,8 +584,16 @@ def run_reviews(session: Session, url: str, args: argparse.Namespace) -> RunOutc
     response, so there is nothing for a second worker to fetch — and the
     session is kept for the whole run, since the token belongs to it.
 
-    Page 1 comes out of the app page's own payload — no second request — and
-    every page after it from `batchexecute`. Both are the same structure.
+    The app page is fetched first, but only to establish that the app exists
+    in this market (Play answers a missing id with a clean 404) and to put the
+    session on Play's own origin for the POSTs that follow. EVERY page of rows
+    comes from `batchexecute`, page 1 included.
+
+    Up to 0.2.0 page 1 was taken from the reviews the app page embeds. Those
+    are in relevance order whatever `--sort` says and carry no continuation
+    token, so page 2 was requested without one, came back as the start of the
+    list again, and was mostly eaten by dedupe — 20 / 1 / 19 rows over three
+    pages in one reported run, and "end_of_listing" after one page in another.
     """
     out = RunOutcome()
     package = parser.package_from_url(url)
@@ -589,7 +603,7 @@ def run_reviews(session: Session, url: str, args: argparse.Namespace) -> RunOutc
 
     try:
         html, status, state, reason = fetch_page(session, url, "reviews", 1,
-                                                 "reviews_p1")
+                                                 "reviews_app")
     except UsageError:
         raise
     except Exception as exc:
@@ -600,11 +614,6 @@ def run_reviews(session: Session, url: str, args: argparse.Namespace) -> RunOutc
         return out
 
     seen: set = set()
-    records = parser.embedded_reviews(html)
-    out.rows.extend(_review_rows(records, package, args, page_num=1,
-                                 offset=0, seen=seen))
-    out.pages_completed = 1
-
     token: Optional[str] = None
     endpoint = parser.batchexecute_url(args.hl, args.gl)
     while out.pages_completed < args.pages:
@@ -743,7 +752,12 @@ def main(launch: Callable[[argparse.Namespace, Optional[str]], Any],
         print(f"[error] {_redact(str(exc))}", file=sys.stderr)
         return EXIT_USAGE
     if pool:
-        print(f"[info] exit {mask(pool.current)} (rotation: {pool.rotate})",
+        # A reviews run keeps one session for its whole length — the
+        # continuation token belongs to it — so a per-page policy does not
+        # apply there and the log must not claim it does.
+        rotation = ("one exit for the run: the review token belongs to the "
+                    "session" if mode == "reviews" else pool.rotate)
+        print(f"[info] exit {mask(pool.current)} (rotation: {rotation})",
               file=sys.stderr)
 
     unpaginated = page_flow.warn_unpaginated(mode, args.pages)

product_parser.py

@@ -68,13 +68,18 @@ class names are build hashes.
 
 Reviews come from one structure reached two ways
 ------------------------------------------------
-The first page of an app's reviews is embedded in the app page's own payload.
-Every page after it comes from `/_/PlayStoreUi/data/batchexecute`, RPC
-`UsvDTd`, which answers a plain POST from a datacentre address with no
-cookies and no key. Page 2 shares none of page 1's ids and every page carries
-a continuation token, so the mode is bounded by `--pages` rather than by the
-store. Both routes are the same 17-slot record, so `parse_review_record()`
-serves both and they cannot drift.
+Every page of `--mode reviews` comes from `/_/PlayStoreUi/data/batchexecute`,
+RPC `UsvDTd`, which answers a plain POST from a datacentre address with no
+cookies and no key. Each response carries a continuation token, and the next
+page is asked for WITH it — page 2 shares none of page 1's ids — so the mode
+is bounded by `--pages` rather than by the store.
+
+The app page also embeds a first handful of reviews in its own payload, in
+the same 17-slot record. They are NOT page 1 of a reviews run, though they
+look like it: they come in the store's "most relevant" order whatever
+`--sort` asks for, and they carry no continuation token, so a run that
+started from them asked for page 2 without one and got the start of the list
+again. `parse_review_record()` still reads both, so the two cannot drift.
 """
 
 import html as _html
@@ -1139,21 +1144,32 @@ def _version(node: Any) -> Optional[str]:
 # to fail cheaply; `--reviews-per-call` raises it.
 REVIEWS_PER_CALL = 200
 
-# Play's sort key. There is exactly ONE, and that is a measurement rather
-# than a design decision: the store's front end distinguishes "most
-# relevant" (1), "newest" (2) and "rating" (3), and in THIS request shape
-# only 2 returns anything at all. Measured 2026-09-22 on two apps, 40
-# requested each: sort 1 -> 0 reviews and no token, sort 2 -> 40 and a token,
-# sort 3 -> 0 and no token.
+# Play's sort keys, as its own front end sends them. All three work.
 #
-# So this repo does not implement the other two orderings. That is a TODO
-# about this request shape, not a limitation of the store — the front end
-# plainly gets them — and the flag offers only what was measured to work,
-# because a `--sort` with three values of which two silently return an empty
-# run is worse than a flag with one.
-REVIEW_SORTS = {"newest": 2}
+# The key goes in the SECOND slot of the sort block, after a constant 2 the
+# front end sends on every request: `[2, <sort>, [count, null, token], null,
+# []]`. Measured 2026-10-22 on com.whatsapp, 20 asked for each:
+#
+#     sort 1  relevant   20 reviews, the store's relevance order
+#     sort 2  newest     20 reviews, posted_at strictly descending
+#     sort 3  rating     20 reviews, grouped by score
+#
+# and `newest` stays in order ACROSS a continuation token: 40 + 40 reviews,
+# no id shared, dates non-increasing over the page boundary.
+#
+# Up to 0.2.0 the key was sent in the FIRST slot. Sort 2 there still returned
+# rows — in relevance order — and sorts 1 and 3 returned nothing at all, which
+# was written up as "the store only answers newest". It was a measurement of
+# the wrong request, not of the store; the correction is the reason for the
+# table above.
+REVIEW_SORTS = {"relevant": 1, "newest": 2, "rating": 3}
 DEFAULT_REVIEW_SORT = "newest"
 
+# The constant the front end sends ahead of the sort key. Named so the shape
+# above is readable at the call site; its meaning is not documented anywhere
+# and nothing here depends on knowing it.
+_SORT_BLOCK_HEAD = 2
+
 # Where each column sits in a review record. Identical in both routes,
 # because they are the same structure.
 REVIEW = {
@@ -1175,16 +1191,21 @@ def build_reviews_request(package_id: str, count: int = REVIEWS_PER_CALL,
     Returned as a dict for the caller to urlencode, so no engine has to know
     the wire format and every engine sends the identical thing.
 
-    There is deliberately no score filter. The slot for one exists in the
-    payload and setting it does NOTHING: measured 2026-09-22, asking for
-    score 1 and score 5 each returned forty reviews spanning all five scores.
-    Shipping a `--min-score`-style filter here would be shipping a flag that
-    reports success and changes no row.
+    This repository does not implement a score filter. The store's front end
+    offers one; the forms tried in the sort block's fourth slot (2026-10-22,
+    correct shape) either changed nothing or returned no rows, so no flag is
+    offered until a form is measured to filter.
+
+    `token` is the continuation token from the PREVIOUS response. Leaving it
+    None asks for the start of the list, which is right for page 1 and wrong
+    for every page after it.
     """
-    sort_key = REVIEW_SORTS.get(sort, REVIEW_SORTS[DEFAULT_REVIEW_SORT])
+    if sort not in REVIEW_SORTS:
+        raise ValueError(f"unknown review sort {sort!r}; "
+                         f"expected one of {sorted(REVIEW_SORTS)}")
     pagination = [count, None, token]
     inner = [None, None,
-             [sort_key, None, pagination, None, []],
+             [_SORT_BLOCK_HEAD, REVIEW_SORTS[sort], pagination, None, []],
              [package_id, 7]]
     payload = [[[REVIEWS_RPC, json.dumps(inner, separators=(",", ":")),
                  None, "generic"]]]
@@ -1240,11 +1261,11 @@ def reviews_from_payload(payload: Any) -> Tuple[List[list], Optional[str]]:
 
 
 def embedded_reviews(markup: str) -> List[list]:
-    """The first page of reviews, which the app page carries in its own payload.
+    """The handful of reviews the app page carries in its own payload.
 
-    So `--mode app` gets reviews without a second request, and `--mode
-    reviews` starts from the same structure it will then page through. One
-    parser for both is what keeps the two routes from drifting.
+    Read to confirm an app page was served with its reviews block, not as
+    page 1 of `--mode reviews`: these are in relevance order whatever `--sort`
+    says and carry no continuation token (see the module docstring).
     """
     out: List[list] = []
     for _key, data in iter_data_blocks(markup):