From 14c1b438281cae0b825180c70becaec2f05b5479 Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:41:50 +0000 Subject: [PATCH] --allow-empty writes an empty file only for a genuine empty result A finished run whose answer was nothing writes [], a header-only CSV and a sidecar with status "empty" (was "failed"), and exits 4. A blocked, never-obtained or parser-failure run keeps its exit code and no longer replaces an earlier output with an empty file; that output and its sidecar are kept byte for byte. With nothing earlier at --out, such a run writes only a sidecar with status "blocked" or "failed". diff_runs.py compares an "empty" run like a complete one. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 24 +++++++++++ README.md | 17 +++++++- diff_runs.py | 4 +- output_writer.py | 54 +++++++++++++++++++----- smoke_test.py | 105 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 191 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4755c3d..01c966a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,30 @@ rather than leaving anyone to discover it from their own output. ## [Unreleased] +> **Behaviour change.** `--allow-empty` now writes an empty file only for a +> genuine empty result, and its sidecar says `status: empty` (was +> `failed`). A blocked or failed run with `--allow-empty` no longer +> replaces an earlier output with an empty file. It keeps its exit code +> (3 or 5) and leaves that output and its sidecar untouched. + +### Fixed + +- **`--allow-empty` could destroy the output it exists to protect.** With + the flag, a blocked run or a run that never obtained its content wrote + an empty JSON and CSV over the previous good result, plus a sidecar + saying `failed`. A genuinely empty, complete run was labelled `failed` + too, so `diff_runs.py` refused it. Now: + - A finished run whose answer was nothing writes `[]`, a header-only + CSV and a sidecar with `status: empty` and its `stop_reason`. It + exits 4. + - Blocked (3), never obtained or parser failure (5) and partial (6) keep + their exit codes. No empty row file is written for them, and an + earlier output at the same `--out` is kept byte for byte, sidecar + included. + - When nothing earlier exists, a blocked or failed `--allow-empty` run + writes only a sidecar, with `status: blocked` or `failed`. + - `diff_runs.py` compares an `empty` run like a complete one. + ## [0.5.0] — 2026-10-15 > **Behaviour change — exit codes.** A page whose navigation never returned diff --git a/README.md b/README.md index 471c7e0..3a1f618 100644 --- a/README.md +++ b/README.md @@ -352,11 +352,24 @@ Browser allows one live connection per profile. | `6` | partial — some page failed or was never reached; `pages_failed` and `pages_unattempted` in the sidecar say which | **A run that finds nothing writes nothing**, so last night's good output -survives a bad night. `--allow-empty` opts out. Every run writes -`.meta.json` beside its data with `status`, `stop_reason`, which pages +survives a bad night. Every run that writes data writes +`.meta.json` beside it with `status`, `stop_reason`, which pages failed by number, `dom_confirm`, `sponsored_rows` and `unauthorised_redirect`. +**`--allow-empty` is for a genuine empty result only.** With it, a run +that finished and found nothing writes an empty JSON (`[]`), a CSV holding +only its header, and a sidecar with `status: empty` and its `stop_reason`, +and still exits `4`. It never turns a blocked or failed run into an empty +one. A blocked run still exits `3`. A run whose content was never obtained, +or whose parser read nothing from a page that stated rows, still exits `5`. +A run that saved rows and then stopped early still exits `6`. For the +zero-row failures no empty row file is written, and an earlier output at +the same `--out` is left untouched, sidecar included. Only when nothing +earlier exists there does a blocked or failed `--allow-empty` run write a +sidecar on its own, with `status: blocked` or `failed`. `diff_runs.py` +compares an `empty` run like a complete one. + `diff_runs.py` compares two runs by `sku` and refuses to compare runs that are not both `complete`, because a partial run's unfetched pages would otherwise read as delisted products. It compares product listings only, and diff --git a/diff_runs.py b/diff_runs.py index d1e2ef8..44b835e 100644 --- a/diff_runs.py +++ b/diff_runs.py @@ -293,7 +293,9 @@ def _check_comparable(args) -> bool: f"{label} ({path}) is a {mode!r} run, which is not one row " f"per sku. This tool diffs one row per sku, so there is " f"nothing here it can compare.") - if status != "complete": + # "empty" is a finished run whose answer was nothing (written only + # with --allow-empty): complete, so it compares like one. + if status not in ("complete", "empty"): problems.append( f"{label} ({path}) was a {status!r} run — stopped after " f"{meta.get('pages_completed')} of {meta.get('pages_requested')} " diff --git a/output_writer.py b/output_writer.py index f7b00f7..b3ac5e9 100644 --- a/output_writer.py +++ b/output_writer.py @@ -778,9 +778,11 @@ def finish_run(rows: Sequence[Any], out_prefix: str, fmt: str, Shared by all three browser engines so the status/exit-code mapping cannot drift between them. - The metadata sidecar is written ONLY when the row file was written. - Otherwise a failed run would leave a "status": "failed" sidecar next to - the previous run's still-intact good output (which `save` deliberately + The metadata sidecar is written when the row file was written, and + for a blocked or failed `--allow-empty` run only when no earlier row + file exists at that path. Otherwise a failed run would leave a + "status": "failed" sidecar next to the previous run's still-intact good + output (which `save` deliberately does not overwrite) — the two files would contradict each other, and diff_runs.py would refuse to compare data that is in fact fine. """ @@ -800,13 +802,45 @@ def finish_run(rows: Sequence[Any], out_prefix: str, fmt: str, complete = stop_reason in COMPLETE_STOP_REASONS and not pages_failed row_cls = ROW_CLASS_BY_MODE.get(mode, Product) write_stats: dict = {} - rc = save(rows, out_prefix, fmt, allow_empty=allow_empty, row_cls=row_cls, - stats=write_stats) - wrote_output = bool(rows) or allow_empty - - if wrote_output: - status = "complete" if (rows and complete) else ( - "partial" if rows else "failed") + # `--allow-empty` writes an empty file ONLY for a genuine empty result: + # the run finished, nothing stood in its way, and the answer was + # nothing. A blocked or failed run (a parser that found nothing on a + # page stating rows included: it reaches here with a failed page) is + # not an answer, so the flag never writes an empty file over a + # previous good result for one, and never turns its exit code into 4. + genuine_empty = not rows and complete and not blocked + failure_sidecar = False + if rows or genuine_empty: + rc = save(rows, out_prefix, fmt, allow_empty=allow_empty, row_cls=row_cls, + stats=write_stats) + wrote_output = bool(rows) or allow_empty + else: + rc = EXIT_FETCH_FAILED # replaced below by blocked/failed + wrote_output = False + kept = [f"{out_prefix}{ext}" for ext in (".json", ".csv") + if os.path.exists(f"{out_prefix}{ext}")] + if kept: + print(f"[!] Nothing was written: this run was " + f"{'blocked' if blocked else 'a failure'} ({stop_reason}), so " + f"the earlier output ({', '.join(kept)}) and its sidecar " + f"are kept as they were.") + elif allow_empty: + # Nothing to preserve, so the sidecar can say what happened + # without contradicting an earlier good result. No row file. + failure_sidecar = True + print(f"[!] --allow-empty writes an empty file only for a " + f"genuine empty result. This run was " + f"{'blocked' if blocked else 'a failure'} ({stop_reason}): no " + f"row file was written, only {out_prefix}.meta.json.") + + if wrote_output or failure_sidecar: + if rows: + status = "complete" if complete else "partial" + elif genuine_empty: + # A finished run whose answer was nothing: complete, and empty. + status = "empty" + else: + status = "blocked" if blocked else "failed" # UNDER the caller's `extra`, never over it. A collision here would # be housekeeping silently dropping something the engine measured # about the site, which is the more valuable of the two. diff --git a/smoke_test.py b/smoke_test.py index ce7b9f0..4166431 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -3298,6 +3298,111 @@ def test_a_url_from_env_does_not_turn_a_store_lookup_into_a_listing(): ".env.example ships an active WOOLWORTHS_URL") +def _allow_empty_contract(O, row_cls): + """(label, got, want) for every outcome `--allow-empty` must keep apart. + + The flag writes an empty file, with a CSV header and a sidecar saying + `status: empty`, ONLY for a genuine empty result: a finished run whose + answer was nothing (exit 4). Blocked (3), never-obtained or parser + failure (5) and partial (6) keep their exit codes, and an earlier good + output is never overwritten for them, rows or sidecar. + """ + import types + import diff_runs + out = [] + good = [row_cls(sku="A1", title="yesterday")] + + def run(prefix, rows, allow, **kw): + base = dict(blocked=False, stop_reason="completed", pages_requested=1, + pages_completed=1, start_url="u", final_url="u", + mode=O.UNIQUE_BY_SKU_MODES[0]) + base.update(kw) + with open(os.devnull, "w") as sink, redirect_stdout(sink): + return O.finish_run(rows, prefix, "both", allow, **base) + + def snapshot(prefix): + return {ext: open(prefix + ext, "rb").read() + for ext in (".json", ".csv", ".meta.json") + if os.path.exists(prefix + ext)} + + failures = ( + ("blocked", dict(blocked=True, stop_reason="blocked", + pages_completed=0), O.EXIT_BLOCKED, "blocked"), + ("never obtained", dict(stop_reason="page_failed", pages_completed=0, + pages_failed=[1]), 5, "failed"), + ("a parser failure on a served page", + dict(stop_reason="completed", pages_failed=[1]), 5, "failed"), + ) + with tempfile.TemporaryDirectory() as d: + # A genuine empty result. + p = os.path.join(d, "empty") + rc = run(p, [], True) + out.append(("genuine empty + --allow-empty: exit", rc, + O.EXIT_NO_PRODUCTS)) + out.append(("...the JSON is []", + json.load(open(p + ".json", encoding="utf-8")), [])) + header = open(p + ".csv", encoding="utf-8").read().splitlines() + out.append(("...the CSV is its header and nothing else", + (len(header), "sku" in (header[0] if header else "")), + (1, True))) + meta = json.load(open(p + ".meta.json", encoding="utf-8")) + out.append(("...the sidecar says empty, why, and 0 rows", + (meta.get("status"), meta.get("stop_reason"), + meta.get("products")), ("empty", "completed", 0))) + p2 = os.path.join(d, "empty_plain") + rc = run(p2, [], False) + out.append(("genuine empty without the flag: exit 4, nothing written", + (rc, snapshot(p2)), (O.EXIT_NO_PRODUCTS, {}))) + + # An empty run compares like a complete one. + g = os.path.join(d, "good0") + run(g, good, False) + with open(os.devnull, "w") as sink, redirect_stdout(sink), \ + redirect_stderr(sink): + ok = diff_runs._check_comparable(types.SimpleNamespace( + old=g + ".json", new=p + ".json", force=False)) + out.append(("diff_runs compares a complete run with an empty one", + bool(ok), True)) + + for label, kw, code, status in failures: + # Over a previous good result: nothing at all changes. + p = os.path.join(d, label.replace(" ", "_")) + run(p, good, False) + before = snapshot(p) + rc = run(p, [], True, **kw) + out.append(("%s + --allow-empty: exit" % label, rc, code)) + out.append(("...the earlier output and its sidecar are kept " + "byte for byte" % (), snapshot(p) == before, True)) + # With nothing to preserve: the sidecar alone, with the truth. + q = p + "_fresh" + rc = run(q, [], True, **kw) + meta = (json.load(open(q + ".meta.json", encoding="utf-8")) + if os.path.exists(q + ".meta.json") else {}) + out.append(("%s, nothing earlier: exit, row files, status" + % label, + (rc, os.path.exists(q + ".json"), + os.path.exists(q + ".csv"), meta.get("status")), + (code, False, False, status))) + + # Rows, then a failure: partial whatever the flag says. + p = os.path.join(d, "partial") + rc = run(p, good, True, stop_reason="page_failed", pages_requested=2, + pages_failed=[2]) + meta = json.load(open(p + ".meta.json", encoding="utf-8")) + out.append(("rows then a failure + --allow-empty: exit 6, partial", + (rc, meta.get("status")), (O.EXIT_PARTIAL, "partial"))) + return out + + +@check +def allow_empty_writes_only_a_genuine_empty_result(): + import output_writer as O + bad = ["%s: got %r, want %r" % (label, got, want) + for label, got, want in _allow_empty_contract(O, O.Product) + if got != want] + assert not bad, "; ".join(bad) + + def main() -> int: total = len(PASSES) + len(FAILURES) print()