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
42 changes: 37 additions & 5 deletions .github/ci_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,32 @@
HEX32_ALLOWED = ("sha", "hash", "nonce", "example", "md5", "digest",
"checksum")

SCANNED_SUFFIXES = (".py", ".md", ".txt", ".yml", ".yaml", ".example")
SCANNED_SUFFIXES = (".py", ".md", ".txt", ".yml", ".yaml", ".example",
# Captured pages, run output and fixtures: where a key
# pasted into a payload, or a front-end key in a capture,
# actually arrives.
".json", ".csv", ".html")

# Directory names skipped wherever they sit INSIDE the repository. Matched
# against the path relative to the repo root, never the absolute path: an
# absolute path carries every ancestor, so a clone made under `~/venv/` or
# `/tmp/.venv/` matched the set on every file, scanned nothing, and printed
# "nothing credential-shaped".
SKIPPED_DIRS = {".git", "__pycache__", ".venv", "venv"}


def _is_env_backup(path):
"""`.env.bak`, `.env.local`, `.env.save`, ...: a copy of the real `.env`.

`.env` itself is where a key belongs, is ignored by git, and is not read
here, or every developer with a working setup would fail this check.
Its COPIES are the case that matters: they are made by hand, named
however someone felt like naming them, and one of them, untracked, once
held a live key in a sibling repository. `.env.example` is read through
its suffix like any other shipped file.
"""
name = path.name
return name.startswith(".env.") and name != ".env.example"


# A directory holding `pyvenv.cfg` is a virtualenv, whatever it is called.
Expand All @@ -112,7 +137,7 @@
# into pip's vendored code and flags a 32-hex string in `_elffile.py` as
# key-shaped. Correct about the string, wrong about the file — and it is the
# FIRST thing a new user sees from `python3 smoke_test.py`. A guard people
# have to argue with is one they learn to suppress (CLAUDE.md §22).
# have to argue with is one they learn to suppress.
#
# Deliberately NOT a narrowing of what gets scanned: an untracked file is
# still read, because a key pasted into a scratch file beside the scripts is
Expand Down Expand Up @@ -141,10 +166,12 @@ def _is_in_virtualenv(path):

def scanned_files():
for path in sorted(REPO.rglob("*")):
if not path.is_file() or path.suffix not in SCANNED_SUFFIXES:
if not path.is_file():
continue
if path.suffix not in SCANNED_SUFFIXES and not _is_env_backup(path):
continue
if any(part in {".git", "__pycache__", ".venv", "venv"}
for part in path.parts):
if any(part in SKIPPED_DIRS
for part in path.relative_to(REPO).parent.parts):
continue
if _is_in_virtualenv(path):
continue
Expand Down Expand Up @@ -236,6 +263,11 @@ def secret_check():
failed.append(f"{rel}:{lineno} contains {match[:6]}… — a "
f"32-char hex string, the shape of a 2captcha key")

if scanned == 0:
# A scan that read nothing has proved nothing, and "nothing
# credential-shaped" is then a false statement rather than a pass.
failed.append(f"0 files scanned under {REPO} -- the exclusions "
f"matched everything, so nothing was checked")
if not failed:
print(f"ok {scanned} files scanned, nothing credential-shaped")
return failed
Expand Down
44 changes: 31 additions & 13 deletions .github/workflows/canary.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ jobs:
echo "::error::this was written. The site was reachable, so this IS a"
echo "::error::parser regression."
exit 1 ;;
5) echo "::warning::Remote API error (exit 5). Nothing was tested."
5) echo "::warning::The content was never obtained (exit 5): a navigation that"
echo "::warning::timed out or came back empty. Nothing was tested."
exit 1 ;;
6) echo "::error::Partial run (exit 6) — see the sidecar's stop_reason."
exit 1 ;;
Expand Down Expand Up @@ -126,12 +127,23 @@ jobs:
fail.append("the walk opened %d gap(s): it outran its own harvest, "
"so rows went by unread" % walk["gaps"])

# Every row must carry what identifies it. These come from the
# parser, not from what was posted today, so anything short of 100%
# is a parser fault.
for col in ("sku", "url", "title"):
missing = sum(1 for r in rows if not r.get(col))
if missing:
fail.append("%d of %d rows have no %s" % (missing, len(rows), col))

priced = sum(1 for r in rows if r["price"] is not None)
share = 100.0 * priced / len(rows) if rows else 0
# 95%-100% across 14 captures; 90 is below every measured value.
if share < 90:
fail.append("only %.0f%% of rows carry a price (floor 90%%, measured "
"95-100%%)" % share)
# A floor on the PARSER, not on the day's posting mix. Free items and
# "make offer" adverts genuinely carry no price, and a 90% floor went
# red on 4 of 32 scheduled runs (79-90%) with a working parser. A
# broken price read takes this near 0, so 60% still catches it.
if share < 60:
fail.append("only %.0f%% of rows carry a price (floor 60%%; 79-100%% "
"on scheduled runs, 2026-09-14 to 2026-10-14)" % share)

# The served batch is where structured data lives at all. Measured
# 66%-99% where the category publishes any; this URL measured 89%.
Expand Down Expand Up @@ -196,21 +208,27 @@ jobs:
run: |
code="${{ steps.served.outputs.exit_code }}"
if [ "$code" != "0" ]; then
echo "::warning::The one-batch run exited $code, so the served-list"
echo "::warning::path was not checked. The walking run above stands."
exit 0
# Red, not a warning: the served list and its JSON-LD alignment
# are checked ONLY here, so a run that did not produce them has
# tested nothing this step exists for.
echo "::error::The one-batch run exited $code, so the served list was"
echo "::error::not checked (3 blocked, 4 zero rows, 5 never obtained)."
exit 1
fi
python - <<'PY'
import json, sys

rows = json.load(open("canary_served.json"))
fail = []

# Measured 2026-09-14: 353 rows for this URL from one served
# response, and 294-358 across the large-area captures.
if len(rows) < 250:
fail.append("only %d rows from one served response; 353 measured, "
"294-358 across captures" % len(rows))
# How many results ONE served response holds varies with the hour:
# 353 on 2026-09-14, 294-358 across captures, and 243 at 06:49 UTC
# on 2026-10-15, when a floor of 250 went red with every row parsed
# (237 of 239 structured). The floor is there to catch a parser that
# reads a handful of rows or none, not the day's volume.
if len(rows) < 150:
fail.append("only %d rows from one served response; 243-358 "
"measured" % len(rows))

enriched = sum(1 for r in rows if r["price_source"] == "jsonld")
share = 100.0 * enriched / len(rows) if rows else 0
Expand Down
20 changes: 20 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,26 @@ live_results.zip
*_debug.html
*_debug.png
*.html.page*
# `--dump-html X --pages 3` writes X.page2 and X.page3 whatever X is called,
# and *.json does not cover `out.json.page3`.
*.page[0-9]*
# A raw page saved at the top of the tree. Nothing shipped is HTML.
/*.html

# --- working directories, by KIND rather than by one chosen name -----------
# A capture or a run's output dropped into a scratch directory nobody listed
# is one `git add -A` from a commit.
live/
out/
output/
result/
results/
run/
runs/
tmp/
scratch/
dump/
dumps/

# --- captures kept for parser work ---------------------------------------
# The smoke suite's fixtures are committed (trimmed, inside smoke_test.py);
Expand Down
58 changes: 57 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@ notes say so first, because nobody should discover that from their output.
> script of yours passes 0, pass **1**: the flag counts ATTEMPTS, so 1 means
> "try once and do not retry".

> **Behaviour change:** `--pages 0`, `--concurrency 0`, and a negative
> `--delay`, `--retry-delay` or `--proxy-block-retries` are now usage errors
> (exit 2) in all three engines, and a negative `--retries` or
> `--retry-delay` in `scraper_api_client.py`. They used to be accepted: a
> negative delay raised mid-run, and a negative `--proxy-block-retries`
> skipped every fetch.

> **Behaviour change:** `diff_runs.py` now refuses (exit 2) to compare two
> runs whose sidecars name different listings — a different area, category
> or filter in the URL — because every row would read as added or removed.
> `--force` compares them anyway.

### Added

- **CSV cells that a spreadsheet would run as a formula are prefixed with an
Expand All @@ -32,6 +44,50 @@ notes say so first, because nobody should discover that from their output.

### Fixed

- **Selenium and pyppeteer no longer crash without a proxy.** With no
`--proxy` (the default), a page that came back refused or empty, and in
pyppeteer a page that never loaded, reached a retry branch that read the
proxy pool's current exit from a pool that did not exist, and the run
ended with exit 1 instead of 3 or 5. Both now re-fetch through the same
access path, as Playwright always did. Checked by driving each engine's
fetch loop with the driver stubbed out and no pool.
- **At most one paid captcha solve per page, on every call site.** The
captcha handler runs once before a page is classified and once after, and
only the second call was counted, so the first one bought a solve on every
retry and every rotation: 2 with no pool and up to 5 with a pool of four
exits, against a limit of 1, with the solver stubbed. One per-page
allowance is now asked immediately before any purchase and is not reset by
a retry or a rotation. No challenge has ever been observed on Craigslist,
so this was latent.
- **The navigation's HTTP status reaches the page classifier** in the
Playwright and pyppeteer engines. A refusal served with 403, 429 or 503
that still references the site's own assets used to read as "not painted
yet" and be waited on. Selenium's driver exposes no status.
- **The credential scan could read nothing and report a clean tree.** Its
directory exclusions were matched against the absolute path, so a clone
under any directory named `venv` or `.venv` scanned 0 files and passed. It
now matches the path inside the repository, fails when it scanned nothing,
and also reads copies of `.env` (`.env.bak`, `.env.local`, ...) and JSON,
CSV and HTML files.
- **README:** exit `5` means the content was never obtained (not only a
remote API error); `--concurrency` above 1 falls back to 1 with a warning
rather than being refused; `--locale` is Playwright-only; a living check
count was removed. The Scraper API client's `--wait-element` example and
its challenge message described another site.
- **`.gitignore`** covers `--dump-html` page dumps of any name
(`*.page2`), a raw `.html` at the top of the tree, and scratch directories
by kind (`live/`, `out/`, `results/`, `tmp/`, ...).
- **Canary:** the one-batch served-list run now fails the job when it does
not exit 0 (it used to warn and pass, so the served list and its JSON-LD
alignment could go untested on a green badge). Every row must carry
`sku`, `url` and `title`. The price-share floor of the walking run is 60%
rather than 90%: 90% went red on 4 of 32 scheduled runs (79-90%) because of
the day's unpriced adverts, with a working parser. The served-batch row
floor is 150 rather than 250: one response held 243 rows at 06:49 UTC on
2026-10-15, every one parsed.
- Shipped files no longer cite private working documents or their numbered
sections, and a suite check keeps it that way.

- **`--retries 0` fetched nothing and called it a result.** The load loop is
`range(1, retries + 1)`, so 0 never runs it and the engine judged
`about:blank`. Three third-party audits of this family ran exactly that and
Expand Down Expand Up @@ -191,7 +247,7 @@ notes say so first, because nobody should discover that from their output.
diffed against a Toronto one.
- `CONTRIBUTING.md` likewise still described the other site in eight places.
- Three documents were referenced and absent: `TROUBLESHOOTING.md` (now
written), plus `FINDINGS.md` and `CLAUDE.md`, whose references were removed
written), plus two local working notes, whose references were removed
— a local working file has no business being named by a published one.
- `captcha_solver.get_balance` was referenced nowhere. Kept and given a
consumer, because its error branch is the half nobody exercises: a wrong
Expand Down
14 changes: 9 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,9 @@ Read these before concluding something is broken.
**There are no pages.** `?page=2` and `?s=296` return a **byte-identical**
response — same length, same first result. Not an empty set, not an error:
the same page. `--pages` therefore walks the site's own rendered list instead
of fetching more URLs, and `--concurrency` above 1 is refused, because a
worker is only useful if it can be handed an address of its own.
of fetching more URLs, and `--concurrency` above 1 falls back to 1 with a
warning, because a worker is only useful if it can be handed an address of
its own.

**One URL tops out at exactly 10,000 results.** Walked to exhaustion, the list
yields 10,000 distinct ids and then the counter stops. To go further, narrow
Expand Down Expand Up @@ -206,6 +207,7 @@ whether a run crashes or spends money. Differences that are real and measured:
| authenticated `--cdp-endpoint` | yes | **no** — `debuggerAddress` has nowhere for a password | yes |
| authenticated `--proxy` | yes | **no** — credentials are stripped, with a warning | yes |
| JavaScript off over a remote profile | no | no | **yes** — per page |
| `--locale` | yes | no | no |
| status | primary | secondary | secondary; upstream is unmaintained |

That third row matters more than it looks. Over `--cdp-endpoint`, Playwright
Expand Down Expand Up @@ -287,19 +289,21 @@ python playwright_scraper.py --url "https://www.craigslist.org/search/area/newyo
```

Exit codes: `0` ok · `1` crash · `2` bad usage · `3` blocked · `4` zero rows ·
`5` remote API error · `6` partial. A run writes `<out>.meta.json` beside its
`5` the content was never obtained (a navigation that timed out or came back
empty, a dead or unauthenticated proxy, a remote API error) · `6` partial. A run writes `<out>.meta.json` beside its
output recording the status, the stop reason and which batches failed; a
**failed** run writes none, and leaves the previous good output in place.

`diff_runs.py` compares two runs by `sku` and refuses to compare runs that are
not both `complete`.
not both `complete`, or that asked for different listings (area, category or
filters in the URL); `--force` overrides either.

---

## Checks

```bash
python3 smoke_test.py # 653 checks, no network, no browser
python3 smoke_test.py # no network, no browser; prints PASS/FAIL per check
pytest # the same suite, wrapped
```

Expand Down
56 changes: 51 additions & 5 deletions diff_runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import re
import sys
from typing import Dict, List, Optional, Tuple
from urllib.parse import parse_qsl, urlsplit

from output_writer import UNIQUE_BY_SKU_MODES

Expand Down Expand Up @@ -241,6 +242,31 @@ def _run_status(path: str) -> Tuple[Optional[str], Optional[dict]]:
return meta.get("status"), meta


def _query_scope(url: Optional[str]) -> Optional[tuple]:
"""What a run ASKED for: host, path and query parameters, order-free.

Craigslist's area is a path segment and its category and filters are
query parameters, so `/search/area/newyork?cat=sss&max_price=25` and
`/search/area/losangeles?cat=sss` are different listings that share one
hostname, one currency and one mode. Two runs of them diffed against
each other report every row as added or removed. The fragment is dropped:
the site rewrites it as it renders, and it does not change what was asked.
"""
if not url:
return None
parts = urlsplit(url)
host = parts.netloc.lower()
if host.startswith("www."):
host = host[4:]
path = parts.path.rstrip("/") or "/"
return (host, path, tuple(sorted(parse_qsl(parts.query, keep_blank_values=True))))


def _hit_cap(meta: Optional[dict]) -> bool:
walks = (meta or {}).get("walk") or {}
return any(isinstance(w, dict) and w.get("hit_cap") for w in walks.values())


def _check_comparable(args) -> bool:
"""Refuse an assortment diff between runs that are not both complete.

Expand All @@ -253,10 +279,21 @@ def _check_comparable(args) -> bool:
"""
problems = []
modes = {}
scopes = {}
for label, path in (("--old", args.old), ("--new", args.new)):
status, meta = _run_status(path)
if status is None:
continue # no sidecar: nothing to check, see _run_status
scope = _query_scope((meta or {}).get("start_url"))
if scope is not None:
scopes[label] = ((meta or {}).get("start_url"), scope)
if _hit_cap(meta):
# Not a refusal: the rows both runs saw are still comparable. But
# a capped run holds a slice of the listing, so `removed` here can
# mean "outside this run's slice" rather than "delisted".
print(f"[!] {label} ({path}) reached Craigslist's result ceiling: "
f"its rows are a slice of the listing, so `removed` may "
f"include adverts that were simply outside that slice.")
mode = (meta or {}).get("mode")
if mode:
modes[label] = mode
Expand All @@ -276,6 +313,13 @@ def _check_comparable(args) -> bool:
f"{label} ({path}) was a {status!r} run — stopped after "
f"{meta.get('pages_completed')} of {meta.get('pages_requested')} "
f"page(s), reason {meta.get('stop_reason')!r}")
if len(scopes) == 2 and len({v[1] for v in scopes.values()}) > 1:
problems.append(
f"the two runs asked for different listings "
f"({scopes['--old'][0]} vs {scopes['--new'][0]}). The area, the "
f"category and every filter are part of the URL here, so "
f"`added`/`removed` would describe the change of question rather "
f"than the listing.")
if len(set(modes.values())) > 1:
problems.append(
f"the two runs are different modes ({modes}). A listing row and a "
Expand Down Expand Up @@ -331,9 +375,9 @@ def _check_comparable(args) -> bool:
print("[!] Refusing to diff these two runs:")
for line in problems:
print(f" {line}")
print(" Re-run the incomplete side, or pass --force to compare anyway "
"(added/removed will include products that were simply never "
"fetched).")
print(" Re-run the incomplete side (or the same URL), or pass --force "
"to compare anyway (added/removed will include products that were "
"simply never fetched).")
return False


Expand All @@ -359,8 +403,10 @@ def parse_args():
"for a cron job that should only notify on a real diff.")
p.add_argument("--force", action="store_true",
help="Diff even when a run's .meta.json says it was partial "
"or failed. Products never fetched by the short run will "
"appear as added/removed.")
"or failed, or when the two runs asked for different "
"listings (area, category or filters in the URL). "
"Products never fetched by one side will appear as "
"added/removed.")
return p.parse_args()


Expand Down
Loading
Loading