Small fixes

finder/README.md

@@ -0,0 +1,224 @@
+# Finder — property listing scraper
+
+Scrapes Greater-London sale listings from **Rightmove**, **OnTheMarket**, and
+**Zoopla**, recovers each property's true full postcode, and writes a single
+parquet (`data/online_listings_buy.parquet`) that the rest of the app consumes
+(after a separate enrich step — see [Output](#output)).
+
+`main.py` is the only entry point; everything else is library code.
+
+---
+
+## How it works (and why it's careful about postcodes)
+
+Every portal's **search** API exposes only an *outcode*-level address (e.g.
+`"…, London, SW9"`) plus map coordinates — never the full unit postcode. The
+full postcode lives on each listing's **detail page**, so the scraper fetches
+detail pages to recover it, and only trusts a detail postcode when its outcode
+agrees with the coordinate-nearest postcode (so a stale/wrong value can never
+silently relocate a listing). When no trustworthy detail postcode is found, it
+falls back to the coordinate-nearest postcode. See the module docstrings in
+`rightmove.py`, `onthemarket.py`, and `zoopla.py` for the per-portal data model.
+
+Detail fetching is the dominant cost, so it is:
+
+- **cached across runs** — `data/detail_cache/{source}.json` maps listing id →
+  recovered postcode; a re-run only fetches *newly-appeared* listings;
+- **fetched concurrently** for the HTTP portals (Rightmove, OnTheMarket), bounded
+  by a shared global rate limiter so the VPN egress stays polite;
+- **gated and capped** per outcode.
+
+See [Performance](#performance--caching).
+
+---
+
+## Prerequisites
+
+The scraper egresses through a VPN. There are two supported ways to provide it:
+
+- **Shared netns (compose, recommended):** an **external `media_gluetun`**
+  container (qmcgaw/gluetun) must already be running on the host. It is managed
+  by a *different* compose; `finder/docker-compose.yml` attaches to its network
+  namespace via `network_mode: "container:media_gluetun"`.
+- **HTTP proxy (standalone):** reach a Gluetun HTTP proxy at `GLUETUN_PROXY`
+  (default `http://gluetun:8888`), or set `GLUETUN_PROXY=""` for a direct,
+  un-tunnelled connection.
+
+Also required: the ARCGIS postcode parquet at `../property-data/arcgis_data.parquet`
+(override with `ARCGIS_PATH`).
+
+---
+
+## Running
+
+### Docker Compose (recommended — the only way that does Zoopla)
+
+`finder/docker-compose.yml` brings up the scraper plus **FlareSolverr** (which
+solves Zoopla's Cloudflare challenge), both sharing `media_gluetun`'s netns. This
+is the intended production-like path.
+
+```bash
+cd finder
+
+# Start the sidecars (finder stays up via `sleep infinity`).
+docker compose up -d --build flaresolverr finder
+
+# Run scrapes inside the container (uv run uses the image's /opt/venv):
+docker compose exec finder uv run python main.py --source all
+docker compose exec finder uv run python main.py --source zoopla --outcodes SW9 --test
+
+docker compose down
+```
+
+> If a leftover `finder_flaresolverr` container exists from earlier testing,
+> remove it first: `docker rm -f finder_flaresolverr`.
+
+In this setup `GLUETUN_PROXY=""` (the shared netns already tunnels everything),
+`ZOOPLA_FETCHER=flaresolverr`, and `DATA_DIR` / `ARCGIS_PATH` are preset by the
+compose file.
+
+### Standalone (quick Rightmove / OnTheMarket dev runs)
+
+Zoopla needs FlareSolverr, so standalone is for the HTTP portals. You just need
+a venv and VPN reachability.
+
+```bash
+cd finder
+
+# One-time: create the venv from the lockfile.
+uv sync --frozen          # creates .venv with httpx, polars, fake-useragent, …
+
+# Small, safe run into a temp dir (does NOT touch real data/):
+.venv/bin/python main.py --source rightmove --outcodes SW9 \
+  --max-properties-per-source 20 --output-dir /tmp/finder-smoke
+
+# Go direct instead of via the gluetun proxy hostname:
+GLUETUN_PROXY="" .venv/bin/python main.py --source onthemarket --outcodes SW9 \
+  --output-dir /tmp/finder-smoke
+```
+
+(`uv run python main.py …` works too and resolves the env automatically.)
+
+---
+
+## CLI reference (`main.py`)
+
+| Flag | Default | Meaning |
+|------|---------|---------|
+| `--source rightmove,onthemarket` | `all` | Comma-separated portal(s): any of `rightmove`, `onthemarket`, `zoopla`, or `all`. |
+| `--outcodes SW9,E14,BR1` | — | Specific outcodes (must be Greater-London-ish). Otherwise the full London set is loaded from ARCGIS. |
+| `--limit-outcodes N` | — | Cap the number of outcodes (quick smoke). |
+| `--max-properties-per-source N` | — | Stop each source after N transformed listings. |
+| `--output-dir DIR` | `data/` | Where the parquet (and `detail_cache/`) are written. |
+| `--test` | off | ~10 likely-London outcodes, ≤100 listings/source, writes to `data/test/`. |
+
+> **Always pass `--output-dir /tmp/...` for testing** — the default `data/` holds
+> the real listings the app consumes.
+
+### Stopping a run
+
+`Ctrl+C` (SIGINT) — or `docker stop` (SIGTERM) — triggers a **graceful
+shutdown**: every source stops at its next outcode boundary, in-flight delays
+and retry backoffs wake immediately, and the run still persists the detail
+caches and writes the listings collected so far before exiting (code `130`).
+Press `Ctrl+C` a second time to force-quit. See `shutdown.py`.
+
+---
+
+## Sources & what each needs
+
+| Source | Transport | Needs FlareSolverr? | Concurrency | Notes |
+|--------|-----------|---------------------|-------------|-------|
+| Rightmove | plain httpx | no | concurrent detail fetches | main path |
+| OnTheMarket | plain httpx | no | concurrent detail fetches | `__NEXT_DATA__` JSON |
+| Zoopla | browser / FlareSolverr | **yes** (`ZOOPLA_FETCHER=flaresolverr`, default) | serial (browser-bound) | Cloudflare-protected; skipped gracefully if FlareSolverr is unavailable |
+
+Rightmove and OnTheMarket run **concurrently in worker threads**; Zoopla runs on
+the **main thread** (its per-outcode wall-clock guard uses `SIGALRM`, which only
+fires on the main thread). One source failing never kills the others.
+
+---
+
+## Output
+
+Each run writes `<output-dir>/online_listings_buy.parquet`.
+
+A **separate enrich step** (outside `finder/`) turns that into
+`online_listings_buy_enriched.parquet`, which is what the Rust backend actually
+loads (`--actual-listings-path …/online_listings_buy_enriched.parquet` in the
+top-level `docker-compose.yml`). That enrich/scheduling pipeline is **not**
+documented here — only the raw scrape is.
+
+The top-level `docker-compose.yml` (Rust `server`, `frontend`, `pocketbase`,
+`screenshot`) is the **web app**; it is downstream of the scrape and is **not**
+required to run the scraper.
+
+---
+
+## Performance & caching
+
+| Mechanism | Where | Effect |
+|-----------|-------|--------|
+| **Persistent detail cache** | `data/detail_cache/{source}.json` | A listing's postcode never changes, so a re-run reuses cached results and only fetches new listings. Delete this folder to force a full re-fetch. |
+| **Concurrent detail fetches** | Rightmove, OnTheMarket | Detail pages fetched in parallel instead of one-at-a-time. |
+| **Global rate limiter** | `http_client.RATE_LIMITER` | Caps the *combined* request rate across all threads/portals so concurrency stays polite. |
+
+> **Note on the "accurate-pin skip" flag (`RIGHTMOVE_SKIP_DETAILS_FOR_ACCURATE_PINS`):**
+> it is currently a **no-op**. The idea was to skip the detail fetch for listings
+> the search already pins precisely (`location.pinType == "ACCURATE_POINT"`), but
+> Rightmove's live search API does not include `pinType` in the payload (only
+> `latitude`/`longitude`), so nothing is ever skipped. It degrades safely (no
+> accuracy loss) but provides no speed-up today.
+
+---
+
+## Configuration
+
+Environment variables (override the defaults in `constants.py`):
+
+| Variable | Default | Purpose |
+|----------|---------|---------|
+| `DATA_DIR` | `finder/data` | Output root. |
+| `ARCGIS_PATH` | `../property-data/arcgis_data.parquet` | Postcode reference data. |
+| `GLUETUN_PROXY` | `http://gluetun:8888` | HTTP proxy for egress; `""` = direct. |
+| `GLUETUN_CONTROL_URL` | `http://gluetun:8000` | Gluetun control API. |
+| `FLARESOLVERR_URL` | `http://gluetun:8191/v1` | FlareSolverr endpoint (Zoopla). |
+| `ZOOPLA_FETCHER` | `flaresolverr` | `flaresolverr` or `camoufox`. |
+| `ZOOPLA_OUTCODE_TIMEOUT_SECONDS` | `300` | Per-outcode wall-clock budget for Zoopla. |
+| `DETAIL_FETCH_CONCURRENCY` | `8` | Parallel detail fetches (Rightmove/OTM). |
+| `REQUESTS_PER_SECOND` | `10` | Global request-rate cap. Lower it if you see `429`/`403`. |
+| `RIGHTMOVE_SKIP_DETAILS_FOR_ACCURATE_PINS` | `1` | Inert today (see note above). |
+
+Non-env code constants worth knowing (`constants.py` / `onthemarket.py`):
+`RIGHTMOVE_FETCH_DETAILS`, `RIGHTMOVE_MAX_DETAILS_PER_OUTCODE` (4000),
+`OTM_FETCH_DETAILS`, `OTM_MAX_DETAILS_PER_OUTCODE` (400),
+`ZOOPLA_FETCH_DETAILS`, `ZOOPLA_MAX_DETAILS_PER_OUTCODE` (4000).
+
+---
+
+## Tests
+
+`pytest` is not a declared dependency; run it ephemerally with uv (no project
+change needed):
+
+```bash
+cd finder
+uv run --with pytest pytest -q
+```
+
+---
+
+## Repo layout
+
+| File | Responsibility |
+|------|----------------|
+| `main.py` | CLI entry point: parse args, build the postcode index, call `run_scrape`. |
+| `scraper.py` | Orchestration: per-source runners, provider parallelism, cache load/save, merge + write. |
+| `rightmove.py` / `onthemarket.py` / `zoopla.py` | Per-portal search + detail scraping and parsing. |
+| `transform.py` | Raw listing → output schema; postcode trust rules. |
+| `http_client.py` | Shared httpx client, retry/backoff, and the global `RATE_LIMITER`. |
+| `postcode_cache.py` | Persistent (cross-run) detail-cache load/save. |
+| `spatial.py` | Grid spatial index for coordinate → nearest postcode. |
+| `storage.py` | Parquet writer (server-ready column names). |
+| `constants.py` | Tunables and endpoints. |
+| `test_*.py` | Unit tests. |

finder/postcode_cache.py

@@ -0,0 +1,67 @@
+"""Persistent detail-fetch caches that survive across scrape runs.
+
+Each portal recovers a listing's true postcode (Rightmove/OnTheMarket) or full
+geo dict (Zoopla) from its detail page. That value never changes for a given
+listing id, yet the in-memory caches are discarded at the end of every run — so
+each run re-fetches every listing's detail page from scratch. Persisting the
+cache to disk means a steady-state run only fetches NEWLY-appeared listings,
+typically a small fraction of the market, which is the single biggest saving
+for a recurring scrape (and it *reduces* request volume rather than adding to
+it).
+
+The stored values are always JSON-serialisable: Rightmove/OnTheMarket cache
+``str | None`` postcodes; Zoopla caches a flat dict of primitives (or ``None``).
+A cached ``None`` (a listing whose detail page yielded nothing usable) is
+persisted too, so a postcode-less listing is not re-fetched every run.
+"""
+
+import json
+import logging
+import os
+import tempfile
+from pathlib import Path
+
+log = logging.getLogger("rightmove")
+
+
+def load_cache(path: str | Path) -> dict:
+    """Load a persisted detail cache. Returns ``{}`` when absent or unreadable.
+
+    A corrupt or non-object file is treated as empty rather than fatal — a bad
+    cache must never block a scrape; the worst case is re-fetching details."""
+    p = Path(path)
+    if not p.exists():
+        return {}
+    try:
+        with open(p, encoding="utf-8") as fh:
+            data = json.load(fh)
+    except (OSError, ValueError) as exc:
+        log.warning("Could not read detail cache %s: %s", p, exc)
+        return {}
+    if not isinstance(data, dict):
+        log.warning("Detail cache %s is not a JSON object; ignoring", p)
+        return {}
+    log.info("Loaded %d cached detail entries from %s", len(data), p)
+    return data
+
+
+def save_cache(path: str | Path, data: dict) -> None:
+    """Atomically write a detail cache to disk (temp file + ``os.replace``).
+
+    Creates the parent directory if needed. Write failures are logged and
+    swallowed: persisting the cache is an optimisation, never a hard
+    requirement, so a read-only disk must not fail an otherwise-good scrape."""
+    p = Path(path)
+    try:
+        p.parent.mkdir(parents=True, exist_ok=True)
+        fd, tmp = tempfile.mkstemp(dir=str(p.parent), suffix=".tmp")
+        try:
+            with os.fdopen(fd, "w", encoding="utf-8") as fh:
+                json.dump(data, fh)
+            os.replace(tmp, p)
+        finally:
+            if os.path.exists(tmp):
+                os.unlink(tmp)
+        log.info("Saved %d cached detail entries to %s", len(data), p)
+    except OSError as exc:
+        log.warning("Could not write detail cache %s: %s", p, exc)

finder/shutdown.py

@@ -0,0 +1,69 @@
+"""Process-wide cooperative shutdown for the scrapers.
+
+A single :class:`threading.Event` is set the first time the process receives
+SIGINT (Ctrl+C) or SIGTERM. Every scrape loop polls :func:`stop_requested` at
+its outcode/page boundaries and every blocking delay goes through :func:`sleep`,
+which wakes the instant a stop is requested. So Ctrl+C makes each source stop
+*starting* new work and unwind through its normal ``finally`` blocks — detail
+caches are persisted and whatever has been collected so far is still written —
+instead of hanging until the worker threads happen to finish (the orchestrator's
+``ThreadPoolExecutor`` used to block the exit waiting on them) or losing the run
+outright.
+
+A second Ctrl+C escalates to a hard :class:`KeyboardInterrupt` for an impatient
+operator. SIGALRM is deliberately left untouched: Zoopla's per-outcode
+wall-clock guard owns it."""
+
+import logging
+import signal
+import threading
+
+log = logging.getLogger("finder")
+
+_STOP = threading.Event()
+
+
+def stop_requested() -> bool:
+    """True once a shutdown signal has been received."""
+    return _STOP.is_set()
+
+
+def request_stop() -> None:
+    """Ask every scrape loop to stop starting new work."""
+    _STOP.set()
+
+
+def reset() -> None:
+    """Clear the flag — for tests and repeated in-process runs."""
+    _STOP.clear()
+
+
+def sleep(seconds: float) -> None:
+    """Sleep that wakes immediately once a stop is requested.
+
+    Drop-in for ``time.sleep`` in the scrape loops, inter-outcode pauses and
+    retry backoffs so a pending Ctrl+C is never stuck behind a multi-second
+    delay."""
+    if seconds <= 0:
+        return
+    _STOP.wait(seconds)
+
+
+def install_signal_handlers() -> None:
+    """Route SIGINT/SIGTERM to :func:`request_stop` (second signal = hard exit).
+
+    Signal handlers run in the main thread, so this must be called from there."""
+
+    def _handle(signum, _frame):
+        if _STOP.is_set():
+            # Operator pressed Ctrl+C twice: abandon the graceful unwind.
+            raise KeyboardInterrupt
+        log.warning(
+            "Signal %s received; finishing in-flight work and saving partial "
+            "results (press Ctrl+C again to force-quit)...",
+            signal.Signals(signum).name,
+        )
+        _STOP.set()
+
+    for sig in (signal.SIGINT, signal.SIGTERM):
+        signal.signal(sig, _handle)

finder/test_http_client.py

@@ -0,0 +1,90 @@
+"""Tests for the shared request rate limiter (http_client.RateLimiter).
+
+Uses a fake clock so the spacing logic is asserted deterministically rather than
+by sleeping for real."""
+
+import threading
+
+import pytest
+
+import http_client
+import shutdown
+from http_client import RateLimiter
+
+
+class _FakeClock:
+    """A monotonic clock that only advances when sleep() is called."""
+
+    def __init__(self, start=1000.0):
+        self.t = start
+        self.sleeps = []
+
+    def monotonic(self):
+        return self.t
+
+    def sleep(self, seconds):
+        self.sleeps.append(seconds)
+        self.t += seconds
+
+
+def _install(monkeypatch):
+    clock = _FakeClock()
+    monkeypatch.setattr(http_client.time, "monotonic", clock.monotonic)
+    # The limiter waits via the interruptible shutdown.sleep so a Ctrl+C wakes a
+    # worker parked on a reserved rate-limit slot instead of stalling the exit.
+    monkeypatch.setattr(shutdown, "sleep", clock.sleep)
+    return clock
+
+
+def test_first_acquire_does_not_sleep(monkeypatch):
+    clock = _install(monkeypatch)
+    RateLimiter(10).acquire()
+    assert clock.sleeps == []
+
+
+def test_subsequent_acquires_are_spaced_by_the_interval(monkeypatch):
+    clock = _install(monkeypatch)
+    rl = RateLimiter(10)  # 0.1s minimum interval
+    rl.acquire()  # no wait
+    rl.acquire()  # waits ~0.1
+    rl.acquire()  # waits ~0.1
+    assert clock.sleeps == pytest.approx([0.1, 0.1])
+
+
+def test_acquire_after_idle_gap_does_not_sleep(monkeypatch):
+    clock = _install(monkeypatch)
+    rl = RateLimiter(10)
+    rl.acquire()
+    clock.t += 5.0  # plenty of idle time passes
+    rl.acquire()
+    assert clock.sleeps == []  # the slot was already free
+
+
+def test_zero_rate_disables_limiting(monkeypatch):
+    def _boom(_seconds):  # pragma: no cover - must never be called
+        raise AssertionError("disabled limiter must not sleep")
+
+    monkeypatch.setattr(shutdown, "sleep", _boom)
+    rl = RateLimiter(0)
+    for _ in range(100):
+        rl.acquire()
+
+
+def test_concurrent_acquires_are_all_spaced(monkeypatch):
+    # Real clock, tiny rate: N threads hitting acquire() at once must be
+    # serialised so the total wall time is at least (N-1) * interval.
+    rl = RateLimiter(200)  # 5ms interval — fast but measurable
+    barrier = threading.Barrier(8)
+
+    def worker():
+        barrier.wait()
+        rl.acquire()
+
+    start = http_client.time.monotonic()
+    threads = [threading.Thread(target=worker) for _ in range(8)]
+    for t in threads:
+        t.start()
+    for t in threads:
+        t.join()
+    elapsed = http_client.time.monotonic() - start
+    assert elapsed >= 7 * 0.005 * 0.5  # allow scheduling slack, but not ~0

finder/test_main.py

@@ -0,0 +1,57 @@
+"""Tests for the CLI argument parsing in main.py."""
+
+import pytest
+
+import main
+
+
+# ---------------------------------------------------------------------------
+# selected_sources — comma-separated --source values
+# ---------------------------------------------------------------------------
+
+
+def test_all_expands_to_every_source():
+    assert main.selected_sources("all") == ["rightmove", "onthemarket", "zoopla"]
+
+
+def test_single_source():
+    assert main.selected_sources("rightmove") == ["rightmove"]
+
+
+def test_comma_separated_sources():
+    assert main.selected_sources("rightmove,onthemarket") == [
+        "rightmove",
+        "onthemarket",
+    ]
+
+
+def test_whitespace_and_case_are_tolerated():
+    assert main.selected_sources(" Rightmove , ZOOPLA ") == ["rightmove", "zoopla"]
+
+
+def test_order_is_canonical_and_deduplicated():
+    # Caller order ignored, duplicates collapsed: keeps SOURCES order so the
+    # downstream merge/dedup stays deterministic.
+    assert main.selected_sources("zoopla,rightmove,rightmove") == [
+        "rightmove",
+        "zoopla",
+    ]
+
+
+def test_all_anywhere_in_the_list_wins():
+    assert main.selected_sources("rightmove,all") == [
+        "rightmove",
+        "onthemarket",
+        "zoopla",
+    ]
+
+
+def test_empty_value_is_rejected():
+    with pytest.raises(SystemExit):
+        main.selected_sources("   ,  ")
+
+
+def test_unknown_source_is_rejected():
+    with pytest.raises(SystemExit) as excinfo:
+        main.selected_sources("rightmove,foo")
+    assert "foo" in str(excinfo.value)

finder/test_postcode_cache.py

@@ -0,0 +1,58 @@
+"""Tests for the persistent detail-fetch cache (postcode_cache.py)."""
+
+from postcode_cache import load_cache, save_cache
+
+
+def test_save_then_load_round_trips_str_and_none_values(tmp_path):
+    path = tmp_path / "rightmove.json"
+    data = {"123": "SW9 0HD", "456": None, "789": "E20 1FH"}
+    save_cache(path, data)
+    assert load_cache(path) == data
+
+
+def test_save_then_load_round_trips_nested_dicts(tmp_path):
+    # Zoopla caches a flat dict of primitives (or None) per listing id.
+    path = tmp_path / "zoopla.json"
+    data = {
+        "59888978": {
+            "lat": 51.4,
+            "lng": -0.11,
+            "postcode": "BR1 2AB",
+            "uprn": "100023",
+            "number_or_name": "12",
+            "full_address": "12 High St",
+        },
+        "broken": None,
+    }
+    save_cache(path, data)
+    assert load_cache(path) == data
+
+
+def test_load_missing_file_returns_empty(tmp_path):
+    assert load_cache(tmp_path / "does-not-exist.json") == {}
+
+
+def test_load_corrupt_file_returns_empty(tmp_path):
+    path = tmp_path / "corrupt.json"
+    path.write_text("{not valid json", encoding="utf-8")
+    assert load_cache(path) == {}
+
+
+def test_load_non_object_file_returns_empty(tmp_path):
+    path = tmp_path / "list.json"
+    path.write_text("[1, 2, 3]", encoding="utf-8")
+    assert load_cache(path) == {}
+
+
+def test_save_creates_parent_directory(tmp_path):
+    path = tmp_path / "nested" / "dir" / "cache.json"
+    save_cache(path, {"a": "B1 1AA"})
+    assert path.exists()
+    assert load_cache(path) == {"a": "B1 1AA"}
+
+
+def test_save_overwrites_existing(tmp_path):
+    path = tmp_path / "c.json"
+    save_cache(path, {"a": "X1 1AA"})
+    save_cache(path, {"b": "Y2 2BB"})
+    assert load_cache(path) == {"b": "Y2 2BB"}

finder/test_rightmove_concurrency.py

@@ -0,0 +1,165 @@
+"""Tests for Rightmove's accurate-pin skip and concurrent detail fetching."""
+
+import threading
+
+import rightmove
+from rightmove import _needs_detail_fetch, _paginate, _prime_detail_postcodes
+
+
+def _prop(pid, pin=None):
+    loc = {"latitude": 51.5, "longitude": -0.1}
+    if pin is not None:
+        loc["pinType"] = pin
+    return {"id": pid, "location": loc}
+
+
+# ---------------------------------------------------------------------------
+# _needs_detail_fetch — accurate-pin skip
+# ---------------------------------------------------------------------------
+
+
+def test_needs_detail_fetch_skips_only_accurate_pins(monkeypatch):
+    monkeypatch.setattr(rightmove, "RIGHTMOVE_SKIP_DETAILS_FOR_ACCURATE_PINS", True)
+    assert _needs_detail_fetch(_prop("1", "ACCURATE_POINT")) is False
+    assert _needs_detail_fetch(_prop("2", "APPROXIMATE_POINT")) is True
+    assert _needs_detail_fetch(_prop("3")) is True  # no pinType -> fetch
+    assert _needs_detail_fetch({"id": "4"}) is True  # no location -> fetch
+
+
+def test_needs_detail_fetch_disabled_always_fetches(monkeypatch):
+    monkeypatch.setattr(rightmove, "RIGHTMOVE_SKIP_DETAILS_FOR_ACCURATE_PINS", False)
+    assert _needs_detail_fetch(_prop("1", "ACCURATE_POINT")) is True
+
+
+# ---------------------------------------------------------------------------
+# _prime_detail_postcodes — worklist selection + concurrent fetch
+# ---------------------------------------------------------------------------
+
+
+def _record_fetcher(monkeypatch):
+    fetched = []
+    lock = threading.Lock()
+
+    def fake_fetch(_client, pid):
+        with lock:
+            fetched.append(pid)
+        rightmove._detail_postcode_cache[pid] = f"PC-{pid}"
+        return f"PC-{pid}"
+
+    monkeypatch.setattr(rightmove, "_fetch_detail_postcode", fake_fetch)
+    return fetched
+
+
+def test_prime_skips_cached_accurate_and_dedups(monkeypatch):
+    rightmove._detail_postcode_cache.clear()
+    rightmove._detail_postcode_cache["cached"] = "SW9 9ZZ"
+    monkeypatch.setattr(rightmove, "RIGHTMOVE_SKIP_DETAILS_FOR_ACCURATE_PINS", True)
+    fetched = _record_fetcher(monkeypatch)
+
+    props = [
+        _prop("cached", "APPROXIMATE_POINT"),  # already cached -> skip
+        _prop("acc", "ACCURATE_POINT"),  # accurate pin -> skip
+        _prop("a", "APPROXIMATE_POINT"),  # fetch
+        _prop("a", "APPROXIMATE_POINT"),  # duplicate id -> skip
+        _prop("b"),  # no pinType -> fetch
+        {"id": ""},  # no id -> skip
+    ]
+    _prime_detail_postcodes(object(), props, True, 10)
+
+    assert sorted(fetched) == ["a", "b"]
+    assert rightmove._detail_postcode_cache["a"] == "PC-a"
+    assert rightmove._detail_postcode_cache["b"] == "PC-b"
+    rightmove._detail_postcode_cache.clear()
+
+
+def test_prime_respects_detail_cap(monkeypatch):
+    rightmove._detail_postcode_cache.clear()
+    monkeypatch.setattr(rightmove, "RIGHTMOVE_SKIP_DETAILS_FOR_ACCURATE_PINS", True)
+    fetched = _record_fetcher(monkeypatch)
+
+    props = [_prop(str(i), "APPROXIMATE_POINT") for i in range(10)]
+    _prime_detail_postcodes(object(), props, True, 3)
+
+    assert sorted(fetched) == ["0", "1", "2"]
+    rightmove._detail_postcode_cache.clear()
+
+
+def test_prime_is_a_noop_when_disabled_or_cap_zero(monkeypatch):
+    rightmove._detail_postcode_cache.clear()
+
+    def _boom(*_a):  # pragma: no cover - must never be called
+        raise AssertionError("must not fetch when disabled")
+
+    monkeypatch.setattr(rightmove, "_fetch_detail_postcode", _boom)
+    _prime_detail_postcodes(object(), [_prop("a")], False, 10)  # fetch_details off
+    _prime_detail_postcodes(object(), [_prop("a")], True, 0)  # cap 0
+    assert rightmove._detail_postcode_cache == {}
+
+
+# ---------------------------------------------------------------------------
+# _paginate — end-to-end (network stubbed): accurate pins fall back to
+# coordinates, approximate pins use the detail postcode.
+# ---------------------------------------------------------------------------
+
+
+class _StubIndex:
+    def __init__(self, postcode):
+        self._postcode = postcode
+
+    def nearest(self, _lat, _lng):
+        return self._postcode
+
+
+def _full_prop(pid, pin):
+    return {
+        "id": pid,
+        "location": {"latitude": 51.477, "longitude": -0.116, "pinType": pin},
+        "price": {"amount": 500000, "displayPrices": []},
+        "displayAddress": "Caldwell Street, Stockwell, London, SW9",
+        "propertySubType": "Flat",
+        "bedrooms": 2,
+        "bathrooms": 1,
+        "propertyUrl": f"/properties/{pid}",
+    }
+
+
+def test_paginate_uses_detail_for_approximate_and_coords_for_accurate(monkeypatch):
+    rightmove._detail_postcode_cache.clear()
+    monkeypatch.setattr(rightmove, "RIGHTMOVE_SKIP_DETAILS_FOR_ACCURATE_PINS", True)
+
+    raw = [
+        _full_prop("approx", "APPROXIMATE_POINT"),
+        _full_prop("acc", "ACCURATE_POINT"),
+    ]
+    monkeypatch.setattr(
+        rightmove, "_collect_search_props", lambda *a, **k: (raw, len(raw))
+    )
+
+    fetched = []
+
+    def fake_fetch(_client, pid):
+        fetched.append(pid)
+        rightmove._detail_postcode_cache[pid] = "SW9 0HD"
+        return "SW9 0HD"
+
+    monkeypatch.setattr(rightmove, "_fetch_detail_postcode", fake_fetch)
+
+    props, count = _paginate(
+        object(),
+        "1",
+        "SW9",
+        {"channel": "BUY", "transactionType": "BUY", "sortType": "2"},
+        _StubIndex("SW9 1AA"),
+        fetch_details=True,
+        detail_cap=10,
+    )
+
+    assert count == 2
+    assert fetched == ["approx"]  # accurate pin was never fetched
+    by_id = {p["id"]: p for p in props}
+    assert by_id["approx"]["Postcode"] == "SW9 0HD"
+    assert by_id["approx"]["Postcode source"] == "detail_address"
+    # Accurate pin keeps the coordinate-nearest postcode.
+    assert by_id["acc"]["Postcode"] == "SW9 1AA"
+    assert by_id["acc"]["Postcode source"] != "detail_address"
+    rightmove._detail_postcode_cache.clear()

finder/test_scraper_concurrency.py

@@ -0,0 +1,150 @@
+"""Tests for the orchestrator's provider parallelism and cache persistence."""
+
+import threading
+
+import onthemarket
+import rightmove
+import scraper
+import zoopla
+
+
+# ---------------------------------------------------------------------------
+# _run_sources — Zoopla inline, others in threads, failures isolated
+# ---------------------------------------------------------------------------
+
+
+def test_run_sources_runs_every_source_and_isolates_failures():
+    order = []
+    lock = threading.Lock()
+
+    def rm():
+        with lock:
+            order.append("rm")
+
+    def otm():
+        with lock:
+            order.append("otm")
+        raise RuntimeError("otm boom")
+
+    def zoo():
+        with lock:
+            order.append("zoo")
+
+    errors: list[str] = []
+    scraper._run_sources(zoo, [("rightmove", rm), ("onthemarket", otm)], errors)
+
+    assert set(order) == {"rm", "otm", "zoo"}
+    # The failing source is recorded but did not stop the others.
+    assert any("onthemarket" in e and "boom" in e for e in errors)
+    assert not any("rightmove" in e for e in errors)
+
+
+def test_run_sources_records_zoopla_failure():
+    errors: list[str] = []
+
+    def zoo():
+        raise ValueError("zoo down")
+
+    scraper._run_sources(zoo, [], errors)
+    assert any("zoopla" in e and "zoo down" in e for e in errors)
+
+
+def test_run_sources_handles_no_background_runners():
+    ran = []
+    errors: list[str] = []
+    scraper._run_sources(lambda: ran.append("z"), [], errors)
+    assert ran == ["z"]
+    assert errors == []
+
+
+def test_run_sources_handles_zoopla_absent():
+    ran = []
+    errors: list[str] = []
+    scraper._run_sources(None, [("rightmove", lambda: ran.append("rm"))], errors)
+    assert ran == ["rm"]
+    assert errors == []
+
+
+# ---------------------------------------------------------------------------
+# Persistent cache wiring
+# ---------------------------------------------------------------------------
+
+
+def test_seed_and_save_detail_caches_round_trip(tmp_path):
+    rightmove._detail_postcode_cache.clear()
+    rightmove._detail_postcode_cache.update({"111": "SW9 0HD", "222": None})
+
+    scraper._save_detail_caches(["rightmove"], tmp_path)
+    assert (tmp_path / "rightmove.json").exists()
+
+    rightmove._detail_postcode_cache.clear()
+    scraper._seed_detail_caches(["rightmove"], tmp_path)
+    assert rightmove._detail_postcode_cache == {"111": "SW9 0HD", "222": None}
+    rightmove._detail_postcode_cache.clear()
+
+
+def test_seed_detail_caches_tolerates_missing_files(tmp_path):
+    rightmove._detail_postcode_cache.clear()
+    # No file written yet — seeding must not raise and must leave cache empty.
+    scraper._seed_detail_caches(["rightmove"], tmp_path)
+    assert rightmove._detail_postcode_cache == {}
+
+
+# ---------------------------------------------------------------------------
+# run_scrape — full orchestration wiring (sources stubbed, no network)
+# ---------------------------------------------------------------------------
+
+
+def _fake_prop(pid, postcode):
+    # Minimal shape: only the fields _merge_properties / _dedup_key read.
+    return {"id": pid, "Postcode": postcode, "price": 100000, "Bedrooms": 2}
+
+
+def test_run_scrape_runs_all_sources_merges_and_persists_caches(tmp_path, monkeypatch):
+    for module in (rightmove, onthemarket, zoopla):
+        module.detail_cache_snapshot()  # ensure hooks exist
+    rightmove._detail_postcode_cache.clear()
+    onthemarket._detail_postcode_cache.clear()
+    zoopla._detail_cache.clear()
+
+    def fake_rm(outcodes, pc_index, results, errors, cap):
+        results["rightmove"].append(_fake_prop("rm_1", "SW9 0HD"))
+
+    def fake_otm(outcodes, pc_index, results, errors, cap):
+        results["onthemarket"].append(_fake_prop("otm_1", "E1 6AN"))
+
+    def fake_zoo(outcodes, pc_index, pc_coords, results, errors, cap):
+        results["zoopla"].append(_fake_prop("z_1", "BR1 2AB"))
+
+    monkeypatch.setattr(scraper, "_scrape_rightmove", fake_rm)
+    monkeypatch.setattr(scraper, "_scrape_onthemarket", fake_otm)
+    monkeypatch.setattr(scraper, "_scrape_zoopla", fake_zoo)
+    monkeypatch.setattr(scraper, "build_postcode_coords", lambda: {})
+
+    written = {}
+    monkeypatch.setattr(
+        scraper,
+        "write_parquet",
+        lambda props, path: written.update(count=len(props), path=path),
+    )
+
+    result = scraper.run_scrape(
+        ["SW9", "E1", "BR1"],
+        pc_index=None,
+        pc_coords=None,
+        sources=["rightmove", "onthemarket", "zoopla"],
+        output_dir=tmp_path,
+        max_properties_per_source=None,
+    )
+
+    # All three sources ran and their listings were merged + written.
+    assert written["count"] == 3
+    assert result["counts"]["total"] == 3
+    assert result["errors"] == []
+    # Persistent cache files were created for every selected source.
+    for source in ("rightmove", "onthemarket", "zoopla"):
+        assert (tmp_path / "detail_cache" / f"{source}.json").exists()
+
+    rightmove._detail_postcode_cache.clear()
+    onthemarket._detail_postcode_cache.clear()
+    zoopla._detail_cache.clear()

finder/test_shutdown.py

@@ -0,0 +1,130 @@
+"""Tests for cooperative Ctrl+C shutdown across the scrapers."""
+
+import signal
+import threading
+import time
+
+import pytest
+
+import http_client
+import scraper
+import shutdown
+
+
+@pytest.fixture(autouse=True)
+def _clean_stop_flag():
+    """Every test starts (and leaves) the global stop flag cleared."""
+    shutdown.reset()
+    yield
+    shutdown.reset()
+
+
+# ---------------------------------------------------------------------------
+# The flag primitive
+# ---------------------------------------------------------------------------
+
+
+def test_request_and_reset_toggle_the_flag():
+    assert shutdown.stop_requested() is False
+    shutdown.request_stop()
+    assert shutdown.stop_requested() is True
+    shutdown.reset()
+    assert shutdown.stop_requested() is False
+
+
+def test_sleep_returns_immediately_once_stop_requested():
+    shutdown.request_stop()
+    started = time.monotonic()
+    shutdown.sleep(30)  # would hang for 30s without the early wake
+    assert time.monotonic() - started < 1.0
+
+
+def test_sleep_wakes_when_stop_requested_from_another_thread():
+    def _trip():
+        time.sleep(0.05)
+        shutdown.request_stop()
+
+    waker = threading.Thread(target=_trip)
+    waker.start()
+    started = time.monotonic()
+    shutdown.sleep(30)
+    waker.join()
+    assert time.monotonic() - started < 1.0
+
+
+# ---------------------------------------------------------------------------
+# Signal handlers: first signal requests stop, second forces a hard exit
+# ---------------------------------------------------------------------------
+
+
+def test_signal_handler_requests_stop_then_escalates():
+    previous = signal.getsignal(signal.SIGINT)
+    try:
+        shutdown.install_signal_handlers()
+        handler = signal.getsignal(signal.SIGINT)
+        assert callable(handler)
+
+        # First Ctrl+C: graceful stop requested, no exception.
+        handler(signal.SIGINT, None)
+        assert shutdown.stop_requested() is True
+
+        # Second Ctrl+C: escalate to a hard interrupt.
+        with pytest.raises(KeyboardInterrupt):
+            handler(signal.SIGINT, None)
+    finally:
+        signal.signal(signal.SIGINT, previous)
+
+
+# ---------------------------------------------------------------------------
+# HTTP layer bails out instead of issuing more requests
+# ---------------------------------------------------------------------------
+
+
+def test_fetch_with_retry_returns_none_without_touching_client():
+    class _ExplodingClient:
+        def get(self, *_a, **_k):  # pragma: no cover - must never be called
+            raise AssertionError("no request should be made after shutdown")
+
+    shutdown.request_stop()
+    assert http_client.fetch_with_retry(_ExplodingClient(), "http://x") is None
+
+
+# ---------------------------------------------------------------------------
+# A source loop stops at the next outcode boundary
+# ---------------------------------------------------------------------------
+
+
+def _londonish_prop(outcode):
+    return {"id": outcode, "Postcode": f"{outcode} 0HD", "price": 100000, "Bedrooms": 2}
+
+
+def test_scrape_rightmove_stops_after_signal(monkeypatch):
+    visited = []
+
+    class _DummyClient:
+        def close(self):
+            pass
+
+    monkeypatch.setattr(scraper, "make_client", lambda: _DummyClient())
+    monkeypatch.setattr(scraper, "resolve_outcode_id", lambda _client, oc: oc)
+    monkeypatch.setattr(scraper, "DELAY_BETWEEN_OUTCODES", 0)
+
+    def fake_search(_client, _oid, outcode, *_a, **_k):
+        visited.append(outcode)
+        # Operator hits Ctrl+C while the first outcode is being processed.
+        shutdown.request_stop()
+        return [_londonish_prop(outcode)]
+
+    monkeypatch.setattr(scraper, "rightmove_search_outcode", fake_search)
+
+    results = {source: [] for source in scraper.SOURCE_ORDER}
+    errors: list[str] = []
+    scraper._scrape_rightmove(
+        ["SW9", "E1", "BR1"], pc_index=None, results=results,
+        errors=errors, max_properties_per_source=None,
+    )
+
+    # Only the first outcode ran; the loop bailed before the next boundary.
+    assert visited == ["SW9"]
+    assert len(results["rightmove"]) == 1
+    assert errors == []