Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 17 additions & 2 deletions playwright_scraper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 17 additions & 2 deletions puppeteer_scraper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 17 additions & 2 deletions selenium_scraper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions smoke_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading