diff --git a/CHANGELOG.md b/CHANGELOG.md index a84d22e..8abb844 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,29 @@ from a row count, it leads the section in a blockquote. ### Fixed +- **`events_on_page_1` under-reported the listing in `--mode events`.** The + audit's last recommendation asks for a freshness and completeness report + so a consumer can see the date, the walk type, the number of events and + the gaps. The sidecar already carries all four — `finished_at`, `mode` and + `pagination`, `total_events` and `events_on_page_1`, and `pages_failed` + beside `pages_without_rows` — so checking that claim was a matter of + reading a real sidecar rather than building anything. Which is how this + turned up. + + The field was counted over the MERGED rows with `r.page == 1`, and that + asks a different question once the walk has merged: a listing row is + REPLACED IN PLACE by the deeper read from its event page, and the + replacement carries that event page's number. So it counted the events + whose rows happened not to be upgraded. Measured on a real ten-page walk: + **3 reported where page 1 had named 11** — printed beside + `total_events: 23475`, which is exactly the comparison a reader makes to + judge how much of the catalogue a run saw. + + Right in `--mode markets`, where nothing is upgraded, which is why it + looked fine. Counted from page 1's own rows now, in all three engines, + with a check controlled against each engine separately — this is the shape + of mistake that gets fixed in one of three. + - **The new virtualenv check failed a fresh clone on its first command.** Found by doing the thing that keeps finding this class of bug: cloning the published repo the way a stranger does, running `python3 -m venv myenv` in diff --git a/playwright_scraper.py b/playwright_scraper.py index c18915e..a356873 100644 --- a/playwright_scraper.py +++ b/playwright_scraper.py @@ -1503,8 +1503,23 @@ def scrape(args) -> int: # site's own twenty-event cap, not a gap this run failed to close # (§8 — an unknown gap is not a gap). "total_events": total_events, - "events_on_page_1": len({r.event_slug for r in all_rows - if r.event_slug and r.page == 1}), + # Counted from PAGE 1's OWN rows, not from the merged ones. + # + # It used to read `r.page == 1` over `all_rows`, and that is a + # different question once `--mode events` has merged: a listing row + # is REPLACED IN PLACE by the deeper read from its event page, and + # the replacement carries that event page's number. So the field + # counted the events whose rows happened NOT to be upgraded. + # + # Measured on a real 10-page walk: it reported 3 where page 1 had + # named 11 — printed beside `total_events: 23475`, which is exactly + # the comparison a consumer makes to judge how much of the catalogue + # a run saw. Wrong in the one mode where the question matters, and + # right in `--mode markets`, where nothing is upgraded, which is why + # it looked fine. + "events_on_page_1": len({p.event_slug for o in outcomes + if o.page_num == 1 + for p in o.products if p.event_slug}), "inline_payload_rows": sum(1 for r in all_rows if r.data_source != "dom"), "jsonld_confirmed_rows": sum(1 for r in all_rows diff --git a/puppeteer_scraper.py b/puppeteer_scraper.py index c79a58c..c12b946 100644 --- a/puppeteer_scraper.py +++ b/puppeteer_scraper.py @@ -952,8 +952,23 @@ def scrape(args) -> int: "scroll": {o.page_num: o.scroll for o in outcomes if o.scroll}, "rows_new_per_page": fresh_by_batch, "total_events": total_events, - "events_on_page_1": len({r.event_slug for r in all_rows - if r.event_slug and r.page == 1}), + # Counted from PAGE 1's OWN rows, not from the merged ones. + # + # It used to read `r.page == 1` over `all_rows`, and that is a + # different question once `--mode events` has merged: a listing row + # is REPLACED IN PLACE by the deeper read from its event page, and + # the replacement carries that event page's number. So the field + # counted the events whose rows happened NOT to be upgraded. + # + # Measured on a real 10-page walk: it reported 3 where page 1 had + # named 11 — printed beside `total_events: 23475`, which is exactly + # the comparison a consumer makes to judge how much of the catalogue + # a run saw. Wrong in the one mode where the question matters, and + # right in `--mode markets`, where nothing is upgraded, which is why + # it looked fine. + "events_on_page_1": len({p.event_slug for o in outcomes + if o.page_num == 1 + for p in o.products if p.event_slug}), "inline_payload_rows": sum(1 for r in all_rows if r.data_source != "dom"), "jsonld_confirmed_rows": sum(1 for r in all_rows diff --git a/selenium_scraper.py b/selenium_scraper.py index e8a7310..b41456b 100644 --- a/selenium_scraper.py +++ b/selenium_scraper.py @@ -967,8 +967,23 @@ def scrape(args) -> int: "scroll": {o.page_num: o.scroll for o in outcomes if o.scroll}, "rows_new_per_page": fresh_by_batch, "total_events": total_events, - "events_on_page_1": len({r.event_slug for r in all_rows - if r.event_slug and r.page == 1}), + # Counted from PAGE 1's OWN rows, not from the merged ones. + # + # It used to read `r.page == 1` over `all_rows`, and that is a + # different question once `--mode events` has merged: a listing row + # is REPLACED IN PLACE by the deeper read from its event page, and + # the replacement carries that event page's number. So the field + # counted the events whose rows happened NOT to be upgraded. + # + # Measured on a real 10-page walk: it reported 3 where page 1 had + # named 11 — printed beside `total_events: 23475`, which is exactly + # the comparison a consumer makes to judge how much of the catalogue + # a run saw. Wrong in the one mode where the question matters, and + # right in `--mode markets`, where nothing is upgraded, which is why + # it looked fine. + "events_on_page_1": len({p.event_slug for o in outcomes + if o.page_num == 1 + for p in o.products if p.event_slug}), "inline_payload_rows": sum(1 for r in all_rows if r.data_source != "dom"), "jsonld_confirmed_rows": sum(1 for r in all_rows diff --git a/smoke_test.py b/smoke_test.py index 6743f2f..d69510b 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -2026,10 +2026,37 @@ def test_engine_parity(skips): group("The three engines agree — flags, in BOTH directions (§17)") ok = True flagsets = {} + sources = {} for engine in ENGINES: src = open(os.path.join(REPO_ROOT, f"{engine}.py"), encoding="utf-8").read() + sources[engine] = src flagsets[engine] = set(re.findall(r'p\.add_argument\("(--[a-z0-9-]+)"', src)) + # `events_on_page_1` must be counted from PAGE 1's OWN rows. + # + # Counting it over the MERGED rows asks a different question, because + # `--mode events` replaces a listing row in place with the deeper read + # from its event page and the replacement carries that page's number. So + # `r.page == 1` over the merged set counts the events that happened NOT + # to be upgraded. Measured on a real ten-page walk: 3 reported where + # page 1 had named 11, printed beside `total_events: 23475` — which is + # exactly the comparison a reader makes to judge how much of the + # catalogue a run saw. + # + # Checked at the source, because the sidecar is assembled inline in each + # engine rather than in a function a test can call — and checked in all + # three, because this is the shape of mistake that gets fixed in one. + for engine in ENGINES: + src = sources[engine] + expression = re.search(r'"events_on_page_1":(.*?)\n\s*"', + src, re.S) + ok &= check("%s computes events_on_page_1" % engine, + expression is not None) + if expression: + body = expression.group(1) + ok &= check("...from page 1's own outcome, not the merged rows", + "o.page_num == 1" in body and "all_rows" not in body) + # The family contract (§9). Every engine must carry all of these. contract = {"--url", "--pages", "--category", "--format", "--out", "--delay", "--retries", "--retry-delay", "--concurrency", "--proxy",