0.1.4: fixes from the audit (offer queue, ssr filters, facets, diff sidecar, atomic writes)

CHANGELOG.md

@@ -11,7 +11,14 @@ burying it.
 
 ## [Unreleased]
 
-> **Behaviour changes.** CSV output now prefixes a cell that starts with
+## [0.1.4]
+
+> **Behaviour changes for existing users.** `--route ssr` now refuses, with
+> exit 2, any filter other than category, city, remote and keyword (it used
+> to accept them and ignore them). `diff_runs.py` now refuses a run with no
+> sidecar unless `--force` is passed. `--retries 0` is refused.
+>
+> CSV output now prefixes a cell that starts with
 > `=`, `+`, `-`, `@`, a tab, CR or LF with an apostrophe (JSON is
 > unchanged), and the sidecar gains `csv_cells_escaped`. `.env.example`
 > no longer sets `JUSTJOIN_URL`; a `JUSTJOIN_URL` from the environment that
@@ -20,6 +27,20 @@ burying it.
 
 ### Fixed
 
+- **An offer queue no longer ends at the first delisted offer** (all three
+  engines). A 404 on one offer was read as the end of the run.
+- **`--mode offer` works in Selenium and pyppeteer**, and pyppeteer no
+  longer crashes without a proxy pool.
+- **`--route ssr` no longer accepts filters it ignores**, and no longer
+  records a sort, order or total the page does not carry.
+- **`--mode facets` is partial when a group is missing**, and the parity
+  engines now request the categories.
+- **`diff_runs.py` refuses runs without a sidecar**, and tells an absent
+  `filters` key from an empty one. Engines always record `filters`.
+- **The Scraper API client writes a sidecar** and picks the row class by
+  mode (its facets CSV had the wrong header).
+- **Output writes are atomic.** A failed write no longer replaces the
+  previous good JSON, CSV or sidecar with a truncated one.
 - **CSV cells are formula-escaped.** Every string in a row is written by an
   employer, and a spreadsheet runs a cell starting with `=`, `+`, `-` or
   `@` as a formula. Escaped in the CSV only, after list cells are joined;

README.md

@@ -112,9 +112,18 @@ is where the offers endpoint lives.
 This scraper defaults to the endpoint, because it is what the site's own
 front end calls from the browser and it is the only route with pagination.
 That is stated here rather than left to be found. `--route ssr` reads the
-rendered listing page instead: **one page of 100 rows** with a poorer
-schema (no category, no languages, no skill levels), entirely within
-robots.txt.
+rendered listing page instead, entirely within robots.txt: it requests only
+`/job-offers/...` and makes no `/api/` request (checked on 2026-10-07 by
+logging every request of a run). What it gives up:
+
+- **one page of about 50 rows** (it was 100 until the page's payload changed
+  on 2026-10-07; the payload now carries no `meta` and no `totalItems`, so a
+  total is not recorded for this route);
+- a poorer schema (no category, no languages, no skill levels);
+- **four filters only**: `--category`, `--city`, `--remote` and `--keyword`.
+  Every other filter is refused with exit 2 before any request, naming the
+  flag, because the page ignores it and the run would otherwise return an
+  unfiltered page labelled as a filtered one.
 
 ---
 
@@ -377,14 +386,32 @@ fetch that page, or the row fell outside this run's slice of a capped
 result set. `diff_runs.py` warns when either run was capped, and refuses a
 pair that used different modes, sorts, orderings or filters.
 
+It also refuses a run that has **no `.meta.json` beside it**: everything
+those checks read lives in that file, so its absence removes all of them at
+once. `--force` diffs on trust. A sidecar written before filters were
+recorded is compared, with a note that the selections could not be.
+
+A run that fetched an offer queue (`--mode offer`) does not end on an offer
+the board has since delisted: that URL answers 404, is counted in the
+sidecar (`offers_not_found`), and the queue goes on. `--retries 0` is
+refused (exit 2) because it would mean no attempt at all.
+
+`--mode facets` is `partial` (exit 6) when any of its groups is missing from
+the result, naming the groups in the sidecar's `missing_groups`. The Scraper
+API client fetches a single URL, so its facets run is partial by
+construction.
+
 ---
 
 ## Engines
 
 Playwright is primary. Selenium and Puppeteer are parity engines — same
-flags, same modes, same exit codes, same output. Verified on 2026-09-18:
-all three produced **byte-identical rows** (modulo `scraped_at`) and
-identical sidecars for the same command.
+flags, same modes, same exit codes, same output. Verified per mode: on
+2026-09-18 `--mode listings` produced **byte-identical rows** (modulo
+`scraped_at`) and identical sidecars in all three engines. That measurement
+covered listings only; `--mode offer` and the `facets` categories were broken
+in the parity engines until 2026-10-07, when they were fixed and run live in
+all three.
 
 ```bash
 pip install -r requirements.txt -r requirements-playwright.txt   # primary

diff_runs.py

@@ -326,9 +326,10 @@ def _print_summary(result: dict) -> None:
 def _run_status(path: str) -> Tuple[Optional[str], Optional[dict]]:
     """Read the `<out>.meta.json` sidecar beside a run's JSON output.
 
-    Returns (status, meta), or (None, None) when there is no sidecar — which
-    is the normal case for output written before run metadata existed, or by
-    `scraper_api_client.py` (single fetch, no pagination to cut short).
+    Returns (status, meta), or (None, None) when there is no sidecar — output
+    written before run metadata existed, or a sidecar that was deleted or
+    never copied beside the JSON. `_check_comparable` treats that as a reason
+    to refuse, not as a pass.
     """
     meta_path = re.sub(r"\.json$", "", path) + ".meta.json"
     try:
@@ -354,7 +355,25 @@ def _check_comparable(args) -> bool:
     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
+            # No sidecar is NOT "nothing to check". Everything this function
+            # checks — whether the run was complete, which mode it was, how it
+            # was sorted and filtered, whether the result set was capped —
+            # lives in that file, so its absence removes every guard at once
+            # and the diff proceeds exactly as if all of them had passed. Two
+            # bare JSON files were accepted with exit 0, and an unfiltered run
+            # beside a filtered one reported hundreds of `removed`.
+            #
+            # Refused, with --force as the explicit way round. Output written
+            # by scraper_api_client.py used to be the reason for the leniency
+            # (it wrote no sidecar); it writes one now, so that reason is gone.
+            problems.append(
+                f"{label} ({path}) has no .meta.json beside it, so nothing "
+                f"says whether it is complete, which mode it was, or how it "
+                f"was sorted and filtered. Without that, every `removed` line "
+                f"may be an offer that was simply never fetched or fell "
+                f"outside a different selection. Re-run it with a current "
+                f"version, or pass --force to diff it on trust.")
+            continue
         mode = (meta or {}).get("mode")
         if mode:
             modes[label] = mode
@@ -421,8 +440,12 @@ def _check_comparable(args) -> bool:
             sorts[label] = meta["sort"]
         if meta.get("order"):
             orders[label] = meta["order"]
-        if meta.get("mode") == "listings":
-            filters[label] = meta.get("filters") or {}
+        # Only when the run RECORDED its filters. A sidecar that says nothing
+        # about them (written before this field existed) is "unknown", which is
+        # not the same as "none" — treating the two alike either refuses a
+        # comparison that is fine or waves through one that is not.
+        if meta.get("mode") == "listings" and "filters" in meta:
+            filters[label] = meta["filters"] or {}
     if len(set(sorts.values())) > 1:
         problems.append(
             f"the two runs used different orderings ({sorts}). Every query "
@@ -436,6 +459,14 @@ def _check_comparable(args) -> bool:
             f"the two runs used different sort directions ({orders}), which "
             f"on a capped result set means they hold opposite ends of the "
             f"same query.")
+    if len(filters) == 1:
+        # One side recorded its selection and the other did not, so it was not
+        # compared. Said out loud rather than skipped: the diff below is only
+        # as meaningful as the two selections being the same.
+        unknown = "--new" if "--old" in filters else "--old"
+        print(f"[i] {unknown} did not record which filters it used, so the two "
+              f"selections could not be compared. If they differ, `added` and "
+              f"`removed` describe that difference rather than the board.")
     if len(filters) == 2:
         old_f, new_f = filters["--old"], filters["--new"]
         if old_f != new_f:

output_writer.py

@@ -106,8 +106,11 @@
 empty CSV still gets the right header for the mode that produced it.
 """
 
+import contextlib
 import csv
 import json
+import os
+import tempfile
 from dataclasses import dataclass, asdict, field, fields
 from datetime import datetime, timezone
 from typing import Optional, List, Set, Sequence, Any, Tuple, Type
@@ -515,8 +518,62 @@ def _csv_cell(v: Any) -> Tuple[Any, bool]:
     return v, False
 
 
+def _target_mode(path: str) -> int:
+    """The permissions the finished file should have.
+
+    `NamedTemporaryFile` creates 0600 and a rename keeps it, so without this
+    every output would come out readable by its owner only — a cron job
+    writing for another user, or a CI step uploading as a different one,
+    would find the file there and be unable to read it. Keep the mode the
+    previous file had; for a first write use what `open()` would have given
+    (0666 minus the umask).
+    """
+    try:
+        return os.stat(path).st_mode & 0o7777
+    except OSError:
+        mask = os.umask(0)
+        os.umask(mask)
+        return 0o666 & ~mask
+
+
+@contextlib.contextmanager
+def _atomic(path: str, newline: Optional[str] = None):
+    """Write to a temporary file beside `path`, then rename over it.
+
+    Every write here replaces a file a previous run may have left, and the
+    invariant this module protects is that a bad run never destroys last
+    night's good data (`save` refuses to overwrite with an empty result for
+    the same reason). `open(path, "w")` truncates before the first byte is
+    written, so a kill, a full disk or a crash halfway through `json.dump`
+    leaves a SHORTER file where a complete one was — and the sidecar beside
+    it, which a consumer branches on, still describes the old good run.
+
+    `os.replace` is atomic within one filesystem, so the temporary file is
+    created in the target's OWN directory; one under /tmp may be on another,
+    where a rename degrades to an interruptible copy. `fsync` before the
+    rename makes it hold after a power loss and not only after a crash.
+    """
+    directory = os.path.dirname(os.path.abspath(path)) or "."
+    handle = tempfile.NamedTemporaryFile(
+        mode="w", encoding="utf-8", newline=newline, dir=directory,
+        prefix=os.path.basename(path) + ".", suffix=".tmp", delete=False)
+    try:
+        with handle:
+            yield handle
+            handle.flush()
+            os.fsync(handle.fileno())
+        os.chmod(handle.name, _target_mode(path))
+        os.replace(handle.name, path)
+    except BaseException:
+        try:
+            os.unlink(handle.name)
+        except OSError:
+            pass
+        raise
+
+
 def write_json(rows: Sequence[Any], path: str) -> None:
-    with open(path, "w", encoding="utf-8") as f:
+    with _atomic(path) as f:
         json.dump([asdict(r) for r in rows], f, ensure_ascii=False, indent=2)
 
 
@@ -529,7 +586,7 @@ def write_csv(rows: Sequence[Any], path: str, row_cls: Type = JobPosting) -> int
     # The header comes from `row_cls`, not from the first row, so an empty
     # run still writes the columns of the mode that produced it.
     fieldnames = [f.name for f in fields(row_cls)]
-    with open(path, "w", encoding="utf-8", newline="") as f:
+    with _atomic(path, newline="") as f:
         writer = csv.DictWriter(f, fieldnames=fieldnames)
         writer.writeheader()
         escaped = 0
@@ -629,7 +686,7 @@ def write_run_meta(out_prefix: str, meta: dict) -> str:
     both complete, and between runs of different `mode`.
     """
     path = f"{out_prefix}.meta.json"
-    with open(path, "w", encoding="utf-8") as f:
+    with _atomic(path) as f:
         json.dump(meta, f, ensure_ascii=False, indent=2)
     print(f"[+] Wrote run metadata -> {path} (status={meta.get('status')})")
     return path

page_flow.py

@@ -250,6 +250,54 @@ def is_terminal(state: str) -> bool:
     return state in ("not_found", "cap_exceeded")
 
 
+def pages_are_independent(mode: str, queued) -> bool:
+    """True when every queued page is its OWN offer rather than a slice of one
+    result set.
+
+    In `--mode offer` a "page" is one offer fetched from an enumerated list,
+    so nothing one page says — empty, repeated, or 404 — is evidence about
+    the next. In a listing the opposite holds: a page that adds nothing new
+    IS evidence that the listing has ended.
+
+    A single `--url` offer has no queue behind it, so it is not "independent"
+    in this sense: there is nothing to continue to, and its 404 should still
+    be the reason the run stopped.
+    """
+    return mode == "offer" and bool(queued)
+
+
+def queue_decision(state: Optional[str], fresh_rows: Optional[int],
+                   independent: bool) -> Optional[str]:
+    """What a finished page says about the REST of the queue.
+
+    Returns the `stop_reason` that ends the queue, or None to carry on. One
+    function, called from every place an engine finishes a page — the first
+    page, the sequential loop and the concurrent workers — because it used to
+    be re-spelt at each of them, and the copies disagreed in the way that
+    matters:
+
+      * the Playwright loop skipped the terminal check for an offer's 404 and
+        then applied "this page added no new rows, so the listing ended" two
+        lines later, so one delisted offer silently cut the queue short and
+        the run reported `complete` with `stop_reason: no_new_products`;
+      * its concurrent worker set the same "end of listing" event on a 404;
+      * the first page stopped the whole run on a 404, so a sitemap whose
+        first slug had been taken down fetched nothing at all;
+      * the two parity engines had no skip at all.
+
+    `fresh_rows=None` means "do not apply the exhausted-listing rule", which
+    is what the first page wants: its content decides how many pages there
+    are, not whether to stop.
+    """
+    if independent:
+        return None
+    if state is not None and is_terminal(state):
+        return state
+    if fresh_rows is not None and fresh_rows == 0:
+        return "no_new_products"
+    return None
+
+
 # Whether a blocked page is worth re-fetching at all.
 #
 # True, and CONSULTED rather than merely documented — the engines read it,