Skip to content

Keep good output on a failed write; one paid solve per page; report 404s and parser failures by name - #10

Merged
jehrr merged 1 commit into
mainfrom
fix/audit-2026-10-14
Oct 15, 2026
Merged

jehrr merged 1 commit into
mainfrom
fix/audit-2026-10-14

Conversation

@jehrr

@jehrr jehrr commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Problem

Measured on main at c00411d:

  • A failed write destroyed the previous output. write_json, write_csv and the .meta.json sidecar used open(path, "w"). When the dump failed part-way, a 732-byte good out.json was left as 10 bytes of invalid JSON.
  • .env.example broke --club-id / --player-id. It shipped TRANSFERMARKT_URL=<market-values URL>, and the environment fills --url before an engine builds the mode's URL. With a copied example, --mode club-squad --club-id 27 scraped the ranking and labelled the result club 27's squad (5 rows from the suite's real fixture). --mode transfers and --mode player were affected the same way.
  • One blocked page could buy one solve per exit. A stubbed solver with a 3-exit pool and --proxy-block-retries 2 recorded 3 purchases for one page.
  • The engines disagreed about a 404. Playwright retried it as a block (exit 3). Selenium and pyppeteer had no status code and called it empty (exit 4).
  • Other defects:
    • An unknown player id was reported as an empty result.
    • A served page whose rows the parser lost ended as exit 4, "zero rows".
    • A captcha named only by its response header was parsed as an ordinary empty page.
    • --concurrency dropped the page held by a worker that died, and still reported the run complete.
  • CSV formula cells were not escaped, and the sidecar had no escape count.
  • The credential scan reported clean after reading nothing. Its exclusions were matched against the absolute path, so a clone under .../venv/ scanned 0 files and reported clean.
  • Negative counts and delays were accepted (--delay -1, --pages 0, --proxy-block-retries -1, …).
  • Comments, the canary and the review prompt cited private working notes by section number.
  • The README described exit 5 too narrowly. Its products table also contradicted the measured v1.1.0 result.
  • The canary could pass without testing anything. A bare-runner exit 3 ended green with a notice.

Resulting behaviour

Behaviour changes

  • Exit codes:
    • parser_found_nothing (the page shows rows, the parser read none) is exit 5 on page 1 (was 4) and exit 6 on a later page.
    • not_found (the site's own 404 page, the edge's nginx 404, or an unknown player id redirected to the ranking) is exit 4 in every engine. Playwright used to give exit 3.
    • A later page answering 404 is exit 6.
  • Newly rejected values (exit 2, naming the flag):
    • --pages, --concurrency and --proxy-sessions below 1.
    • --delay, --retry-delay and --proxy-block-retries below 0.
    • In scraper_api_client.py: --retries and --retry-delay below 0, and --timeout below 1.
  • File modes: a new file gets 0666 & ~umask and an existing file keeps its mode. Outputs are renamed into place.
  • CSV: a formula-shaped text cell gets a leading ' in the CSV only. csv_cells_escaped in the sidecar counts them, merged under the caller's extra keys.
  • TRANSFERMARKT_URL is commented out in .env.example. A typed builder flag, or a --mode of another page kind, now wins over it. Every entry point captures the typed --url before reading the environment.
  • The canary fails on exit 3/4/5/6. No secret is needed: the bare-runner path read 75 rows over 3 pages on every run from 2026-10-06 to 2026-10-15.
  • Solve budget: each page gets one paid-solve allowance (page_flow.SolveBudget). It is created before the block-retry loop and checked before solve_recaptcha in all three browser engines. The audit's probe now records 1 purchase with a 3-exit pool, and 1 with no pool.
  • Credential scan: exclusions use path.relative_to(REPO).parts, a scan of 0 files fails, and .json/.sh/.toml/.html/.cfg files are scanned too (41 files, was 36).
  • .gitignore: captures are ignored by kind (/*.html except index.html, *.page[0-9]*, live/, out/, captures/).
  • Private-note citations are removed from all tracked files. Released CHANGELOG sections changed only by removing those citations. A new suite check fails if one returns. The release notes for v1.0.0 and v1.1.0 contained none.
  • tests.yml runs ci_checks.py --sample-check instead of an inline copy of it.
  • Repo topic: price-monitoring → sports-analytics.

The 2026-10-04/05 red canaries were the known gallery case. The site's own next-link was the doubled /galerie/{uuid}/page/5//page/2 shape, and the edge answered it with a bare nginx 404. That page, from the run's artifact, is now a fixture. Without a status code it used to read as empty, which a chained Selenium or pyppeteer run would take for the end of the listing.

Evidence

  • Offline suite (PYTHONDONTWRITEBYTECODE=1, __pycache__ cleared):

    • core-only venv: 697 PASS / 0 FAIL
    • playwright venv: 740 / 0
    • pyppeteer venv: 727 / 0
    • selenium venv: 725 / 0
    • In each engine venv the module imports, --pages 0 and --delay -1 give exit 2 naming the flag.
    • pytest -q passes, ci_checks.py --all is OK, and compileall is clean.
  • Controls: each fault below was planted and the suite went RED on the named check, then GREEN once restored.

    • Writer and CSV: a truncating JSON writer; no chmod (0600); a flat 0644 mode; no CSV escape; escaping before the list join; the escape count merged over the caller's extra.
    • Solve budget: the budget made inside the retry loop; the check-before-purchase removed (both structurally, and behaviourally in the playwright venv: 2 purchases); the budget made optional.
    • Flags and environment: a plain-float --delay (selenium); a plain-int --retries (api client); typed_url captured after apply; the env-URL conflict ignored; the URL uncommented in .env.example.
    • Page states: no 404 body marker; no nginx 404 marker; the redirect not checked; parse_failed never set (api client, playwright); a header-only captcha parsed (api client, pyppeteer).
    • Scan, citations, concurrency: scan exclusions on absolute parts; an empty scan reported clean; a planted citation; a dead worker's page dropped (playwright venv).
  • Live, free, from this machine on 2026-10-15 (Playwright engine with the system Chromium, no proxy, no key):

    Command Exit stop_reason
    --mode club-squad --club-id 99999999 4 not_found
    --mode player --player-id 999999999 4 not_found
    --mode player --player-id 418560 0 1 row
    --mode transfers --pages 2 0 20 rows, no_new_products (see below)

    The 404 and redirect shapes in the fixtures were captured the same day.

  • No paid calls, no proxies, no credentials.

Not changed (and why)

  • Canary skip/notice policy: out of scope as a policy question. The canary change here is limited to making a block fail the job.
  • Version bump, tag and release: [Unreleased] now holds these changes and the earlier ones. Owner decision.
  • Shared fetch-loop refactor: the repo still has three per-engine loops. This is a separate decision.
  • AWS WAF voucher vs existing_token, and the cookie domain from awsWafCookieDomainList: needs a live solve control on this site first.
  • Marker hygiene (cf-turnstile, Access Denied, …): a candidate only. It is mitigated by the content-first ordering.
  • CLI flag parity (--category, --chromium-path, --no-autosolve across engines): low, not changed.
  • Site observation, not fixed: on 2026-10-15 /statistik/neuestetransfers served 20 rows with no next-link, and ?page=2 returned the same 20 rows, so --mode transfers ends no_new_products after page 1. This is honest about what the site serves, but it is a change from the measured 25-row paginated listing. It was not in the audit and is left for a separate look.

🤖 Generated with Claude Code

…04s and parser failures by name

- Outputs, the sidecar and diff_runs --out are written atomically (temp file
  in the target's directory, fsync, rename); new files get 0666 & ~umask,
  existing files keep their mode.
- CSV cells starting with = + - @ TAB CR LF are prefixed with ' in the CSV
  only; csv_cells_escaped goes in the sidecar under the caller's extra keys.
- One paid-solve allowance per page in all three browser engines, checked
  before every purchase and not reset by a retry or an exit rotation.
- The site's 404 page, the edge's nginx 404 and an unknown player id
  redirected to the ranking are not_found (exit 4) in every engine; a served
  page whose rows the parser lost is parser_found_nothing (exit 5, or 6
  after rows). A captcha named only by its header is blocked.
- TRANSFERMARKT_URL is commented out in .env.example and no longer overrides
  --club-id/--player-id/--season or a --mode of another page kind.
- Numeric count/delay flags reject values below their floor (exit 2).
- The credential scan judges paths inside the repo and fails on 0 files.
- --concurrency accounts for a page held by a worker that died.
- Capture kinds are gitignored; private-note citations removed, with a
  suite guard; README exit-5 row and products table corrected.
- The canary fails on exit 3 instead of skipping green.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jehrr
jehrr marked this pull request as ready for review October 15, 2026 21:12
@jehrr
jehrr merged commit adc84f8 into main Oct 15, 2026
9 checks passed
@jehrr jehrr mentioned this pull request Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant