diff --git a/CHANGELOG.md b/CHANGELOG.md index 93627ad..84ee585 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,30 @@ does it leads the release notes rather than being discovered from a bill. ## [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.2.0] — 2026-10-15 > **Behaviour changes:** diff --git a/README.md b/README.md index 6a3bb2f..0d916f1 100644 --- a/README.md +++ b/README.md @@ -333,12 +333,25 @@ Exit codes: `0` ok · `1` crash · `2` bad usage · `3` blocked · `4` zero jobs · `5` remote API error · `6` partial. * **A run that finds nothing writes nothing** — last night's good output is - never replaced with `[]`. `--allow-empty` opts out. + never replaced with `[]`. * **Blocked ≠ empty ≠ partial**, three distinct exit codes, and `.meta.json` records `status`, `stop_reason` and *which* pages failed by number. * **A failed run writes no sidecar**, so a `"failed"` file never sits beside - good data. + good data (the one exception is below, and only where no earlier output + exists). +* **`--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. * **An empty CSV still carries its header.** * `diff_runs.py` refuses to compare runs that are not both `complete`, or whose modes differ. diff --git a/diff_runs.py b/diff_runs.py index 9ddba3b..22687eb 100644 --- a/diff_runs.py +++ b/diff_runs.py @@ -270,7 +270,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 on price, so there " f"is 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 f937e63..a093a68 100644 --- a/output_writer.py +++ b/output_writer.py @@ -630,9 +630,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. """ @@ -652,13 +654,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, JobPosting) 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" # Merged UNDER the caller's `extra`: an engine's own facts win a name # collision, and run fields still win over both (see run_meta). if write_stats: diff --git a/smoke_test.py b/smoke_test.py index 559dc3f..57b4ea8 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -40,6 +40,7 @@ import sys import pathlib import tempfile +from contextlib import redirect_stderr, redirect_stdout from dataclasses import asdict, fields HERE = os.path.dirname(os.path.abspath(__file__)) @@ -2720,6 +2721,108 @@ def check_no_private_citations_in_public_files(): scanned > 20) +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 + + +def check_allow_empty_writes_only_a_genuine_empty_result(): + import output_writer as O + for label, got, want in _allow_empty_contract(O, O.JobPosting): + equal(label, got, want) + + CHECKS = [v for k, v in sorted(globals().items()) if k.startswith("check_")]