Skip to content

Audit fixes: a run that fetched nothing no longer reports "0 jobs" - #6

Merged
jehrr merged 3 commits into
mainfrom
audit-2026-10-13
Oct 13, 2026
Merged

jehrr merged 3 commits into
mainfrom
audit-2026-10-13

Conversation

@jehrr

@jehrr jehrr commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

From a third-party audit dated 2026-10-13. Four of its findings were real and are fixed here. Its headline live result was an artefact of its own command — and that artefact is fixed too, because it is what made the misreading possible.

Behaviour change: --retries 0 is now a usage error (exit 2). It counts ATTEMPTS per page load, and zero never entered the load loop.

The one that matters (the audit's P1 — confirmed)

A page the scraper never read reported exit 4, "zero jobs". The chain was four links long and each link looked right on its own:

state `unknown` → should_parse False → outcome still marked `ok`
(it consulted only load_failed and blocked_by) → stop_reason left at
"completed" → zero rows → exit 4

finish_run has had the exit-5 branch since the family unified on it, and nothing ever reached it, because nothing told it the run had failed. An ETL trusting the exit code would conclude the catalogue had emptied.

page_flow.STATE_POLICY now carries obtained per state, and stop_reason_for() is one implementation of a triage the three engines were deriving in seven places between them.

state obtained exit why
content yes 0 rows
empty yes 4 the site served a listing and it matched nothing — an answer
not_found yes 4 a 404 is the site saying definitively the address holds nothing
unknown no 5 nothing can be concluded
parse_error no 5 a served page we could not read is our bug, not an empty catalogue
blocked no 3 unchanged

not_found was in the failure bucket in my first pass. That was wrong — corrected here rather than shipped.

The audit's own command

It ran --retries 0. The loop is range(1, retries + 1), so zero never entered it: goto was not called and the engine classified about:blank — 39 bytes, no markers, no assets. Reproduced exactly on the audited commit, then one variable changed:

command result
the audit's, --retries 0 39 bytes, unknown, exit 4
the same, --retries 1 11,922 bytes, blocked, exit 3 — the site's real refusal, correctly classified
--retries 1 + residential exit, 3 pages 115 rows, exit 0, on 2026-10-13

So the site had not stopped serving this scraper and the parser was not stale. But a flag that silently turns a run into a different run is how a competent reader reached the opposite conclusion, so it is rejected at parse time rather than clamped.

Also fixed

  • --category did nothing in the Scraper API adapter — it set row.category on a dataclass with no such field, a dynamic attribute asdict() never sees. It is a run label and now goes to the sidecar; the adapter writes one at all now.
  • The same adapter returned exit 4 for a page the API served that parsed to nothing — this repo's bug, not an empty catalogue.
  • Donor residue the audit did not reach: the adapter's description and --mode help still described bbb-scraper's profile mode, which does not exist here.
  • Writes were not atomic. open(path, "w") truncates before a byte is written. Worst for <out>.meta.json, the file a consumer branches on. Ported with the mode fix the donor lacks — a naive NamedTemporaryFile leaves every output 0600. Controlled: a 37,872-byte good file survives an interrupted write intact, mode 0664, no temp files left.
  • Python 3.13 in the CI matrix — the badge claimed 3.9–3.13 while CI tested 3.9 and 3.12.

Robots: agreed on the gap, not on the remedy

The audit is right that "reads only routes robots.txt allows" was a claim about behaviour with no mechanism — the word appeared in comments and nowhere in the code.

It is not a runtime gate here, and that is argued rather than deferred: a preflight fetch of robots.txt is itself 403 from a datacentre address, like every route on this site. The gate would either fail closed exactly where the scraper already fails — a second failure mode for no safety gained — or fail open, which is a gate that does not gate.

What makes the claim true is a property of the code, so it is tested as one: every route the parser accepts is allowed, every route the published rules disallow (/u/, /_jobs/, /search, /jobs/signup, /auth/) has no mode. Checked against the live file, fetched through a residential exit and inlined. The docs now say what is guaranteed.

Verification

Every fix controlled in both directions with __pycache__ cleared: the planted fault turns the suite red on the named check, restoring makes it green. 435 checks.

All three engines re-run live afterwards — 52 identical rows each, exit 0 — and the failure paths re-verified: a real block is still exit 3, --retries 0 is exit 2.

Not changed, with reasons

  • The three-engine matrix. The audit suggests narrowing to Playwright. The limits it names are documented in the README already and the engines exist for parity by design; the maintenance cost is real and is tracked as a family-wide refactor, not a deletion.
  • The canary gate. It skips without a proxy secret by design — this site refuses every datacentre address, and a check that is always red teaches everyone to ignore checks.

🤖 Generated with Claude Code

jehrr and others added 3 commits October 13, 2026 11:23
From a third-party audit dated 2026-10-13. Four of its findings were real
and are fixed here; its headline live result was an artefact of its own
command, and that artefact is fixed too, because it is what made the
misreading possible.

THE ONE THAT MATTERS (the audit's P1, confirmed)

  A page the scraper never read reported exit 4, "zero jobs". The chain was
  four links long and each looked right alone: state `unknown` ->
  should_parse False -> an outcome still marked `ok` (it consulted only
  `load_failed` and `blocked_by`) -> stop_reason left at "completed" ->
  zero rows -> exit 4. `finish_run` has had the exit-5 branch since the
  family unified on it (CLAUDE.md §25) and nothing ever reached it, because
  nothing told it the run had failed.

  `page_flow.STATE_POLICY` now carries `obtained` per state, and
  `stop_reason_for()` is one implementation of a triage the engines were
  deriving in SEVEN places between them. `unknown` and `parse_error` exit 5.

  `empty` stays exit 4 — the site served a listing and it matched nothing,
  which is an answer. So does `not_found`: a 404 is the site saying
  definitively that the address holds nothing, and a consumer can conclude
  something about the catalogue from it. (I had it in the failure bucket
  first; that was wrong and is corrected here rather than shipped.)

THE AUDIT'S OWN COMMAND

  It ran `--retries 0`. The load loop is `range(1, retries + 1)`, so zero
  never entered it: `goto` was not called, and the engine classified
  `about:blank` — 39 bytes, no markers, no assets. Reproduced exactly on
  the audited commit. The SAME command with `--retries 1` returns the
  site's real refusal: 11,922 bytes, `blocked`, exit 3. With a residential
  exit: 115 rows over three pages, exit 0, on 2026-10-13.

  So the site had not stopped serving this scraper, and the parser was not
  stale. But a flag that silently turns a run into a different run is how a
  competent reader reached the opposite conclusion, so `--retries` is now
  `page_flow.attempts`: rejected at parse time with a reason, not clamped.
  This is CLAUDE.md §27.9's family-wide defect, which lists this repo.

ALSO FIXED

  * `--category` in the Scraper API adapter set `row.category` on a
    dataclass with no such field — a dynamic attribute `asdict()` never
    sees, so the flag did nothing. It is a RUN label and now goes to the
    sidecar, which is where the browser engines put it. The adapter also
    writes a sidecar at all now, via finish_run.
  * The same adapter returned exit 4 for a page the API SERVED that parsed
    to nothing. That is this repo's bug, not an empty catalogue (§20), and
    it now reports a non-complete stop reason.
  * Donor residue the audit did not reach: the adapter's description and
    `--mode` help still described bbb-scraper's `profile` mode, which does
    not exist here.
  * Writes were not atomic — `open(path, "w")` truncates before a byte is
    written, so an interrupted run left a SHORTER file where a complete one
    was. Worst for `<out>.meta.json`, the file a consumer branches on.
    Ported from rakuten-scraper WITH the mode fix it lacks: a naive
    NamedTemporaryFile leaves every output 0600. Controlled: a 37,872-byte
    good file survives an interrupted write intact, mode 0664, no temp
    files left.
  * Python 3.13 is in the CI matrix; the badge claimed 3.9–3.13 and CI
    tested 3.9 and 3.12.

ROBOTS: AGREED ON THE GAP, NOT ON THE REMEDY

  The audit is right that "reads only routes robots.txt allows" was a claim
  about behaviour with no mechanism — the word appeared in comments and
  nowhere in the code. It is not a runtime gate here, and that is argued
  rather than deferred: a preflight fetch is itself 403 from a datacentre
  address, so the gate would fail closed exactly where the scraper already
  fails, or fail open, which is a gate that does not gate.

  What makes the claim true is a property of the code, so it is tested as
  one: every route the parser ACCEPTS is allowed, every route the published
  rules disallow has no mode, checked against the live file (fetched
  2026-10-13 through a residential exit, inlined per §10). The docs now say
  what is actually guaranteed.

Every fix controlled in both directions with __pycache__ cleared (§22): the
planted fault turns the suite red on the named check, restoring makes it
green. 435 checks. All three engines re-run live afterwards — 52 identical
rows each, exit 0 — and the failure paths re-verified: a real block is still
exit 3, `--retries 0` is exit 2.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014iC5Xk2LeBPv5TRBXJAU8M
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014iC5Xk2LeBPv5TRBXJAU8M
The audit's fifth real finding, and the one that made the other four harder
to diagnose: "тело ответа не было сохранено для последующего анализа".

The blocked path has always written a debug dump. The content-not-obtained
path — `unknown`, `parse_error` — wrote nothing at all, so a failing run
left no artefact and the only evidence of WHY was a log line. That is half
of why the auditor read a never-fetched `about:blank` as the site refusing
us: there was nothing to look at.

All three engines now write `<out>_page{N}_debug.html` on that branch and
log the byte count and the own-asset reference count beside it, which is
what separates "the site served something we could not read" from "nothing
arrived" at a glance.

Controlled: removing the dump from one engine turns the suite red on the
named check; restoring it goes green. 444 checks. All three engines re-run
live — 52 identical rows each, exit 0 — and a blocked run still exits 3 and
writes its dump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014iC5Xk2LeBPv5TRBXJAU8M
@jehrr
jehrr merged commit d92df63 into main Oct 13, 2026
8 checks passed
@jehrr
jehrr deleted the audit-2026-10-13 branch October 13, 2026 12:15
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