From 625b583799f22c2cf91dcbc9922cccd30acfa872 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 18:12:14 +0000 Subject: [PATCH] fix(arxiv_refs): an arXiv outage now turns the workflow red The script already returned 1 on transport failure, but the workflow piped it through tee without pipefail, so the job stayed green. Add pipefail, record the exit status, keep partial progress, then fail the job. Injectable sleep for the retry ladder, ParseError -> ArxivAPIError, unattempted stat, witness test (HTTPError 429 is an API failure, never a non-match or a NOTE-mark). Git history: nothing was NOTE-marked in the 2026-09-14..18 outage window. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016nhQ8REafSSE74S71P4set --- .github/workflows/arxiv_refs.yml | 32 +++++++- scripts/arxiv_refs.py | 42 ++++++++-- scripts/backfill_arxiv_refs.py | 47 ++++++++--- tests/test_arxiv_refs.py | 133 +++++++++++++++++++++++++++++++ 4 files changed, 236 insertions(+), 18 deletions(-) diff --git a/.github/workflows/arxiv_refs.yml b/.github/workflows/arxiv_refs.yml index e561eb8..063dbdc 100644 --- a/.github/workflows/arxiv_refs.yml +++ b/.github/workflows/arxiv_refs.yml @@ -16,6 +16,13 @@ name: arXiv refs backfill # historical backlog over a few nights. Titles arXiv genuinely does not have are # re-attempted each run — the deliberate cost of keeping no per-line failure # state, and bounded by the same cap. +# +# An arXiv *outage* is not a non-match. The script retries 429/406/5xx along a +# back-off ladder honouring Retry-After; a lookup that still gets no answer is +# counted as an API failure, stops the run, is never NOTE-marked, and exits +# non-zero. This job then commits whatever resolved before the failure and +# goes red at the end — it ran green through every night of the 2026-09-14..18 +# outage because a bare `except` turned each 429 into "no unique match". on: schedule: @@ -66,7 +73,7 @@ jobs: DRY_RUN: ${{ inputs.dry_run }} MARK_UNRESOLVED: ${{ inputs.mark_unresolved }} run: | - set -u + set -u -o pipefail args="--limit ${LIMIT:-60}" [ "${DRY_RUN:-false}" = "true" ] || args="$args --write" # Never on the schedule: `inputs.*` is empty on a cron fire, so this @@ -74,9 +81,20 @@ jobs: [ "${MARK_UNRESOLVED:-false}" != "true" ] || args="$args --mark-unresolved" # `-u`: unbuffered, so the log streams while the run is in flight # instead of arriving in one block at the end. - python -u scripts/backfill_arxiv_refs.py $args | tee /tmp/backfill.log + # The exit status is captured, not acted on here: a transport failure + # must still let the refs resolved before it be validated and + # committed. The last step turns it into a red job. + status=0 + python -u scripts/backfill_arxiv_refs.py $args | tee /tmp/backfill.log || status=$? + echo "status=${status}" >> "$GITHUB_OUTPUT" { echo '## arXiv refs backfill' + if [ "$status" -ne 0 ]; then + echo + echo "> **arXiv API failure** — a lookup got no answer after the retry ladder (exit ${status})." + echo "> The run stopped there; nothing was NOTE-marked on a failed lookup." + echo + fi echo '```' cat /tmp/backfill.log echo '```' @@ -117,3 +135,13 @@ jobs: - name: Refresh the dashboard if: steps.backfill.outputs.changed == 'yes' && github.ref == 'refs/heads/main' run: gh workflow run knowledge_board.yml --ref main + + # Last, so partial progress is committed first: an arXiv outage turns the + # job red instead of reporting "N ref-less, 0 resolved" in green. + - name: Fail on arXiv API failure + if: steps.backfill.outputs.status != '0' + env: + STATUS: ${{ steps.backfill.outputs.status }} + run: | + echo "::error::arXiv refs backfill exited ${STATUS} — arXiv API transport failure (see the step summary)" + exit 1 diff --git a/scripts/arxiv_refs.py b/scripts/arxiv_refs.py index 02a781a..ce1dbf1 100644 --- a/scripts/arxiv_refs.py +++ b/scripts/arxiv_refs.py @@ -31,6 +31,7 @@ from __future__ import annotations +import http.client import re import subprocess import sys @@ -69,12 +70,22 @@ ACCEPT = "application/atom+xml" RETRY_DELAYS = (5, 15, 45, 90, 180) RETRY_STATUSES = {406, 429, 500, 502, 503, 504} -TRANSIENT = (urllib.error.URLError, TimeoutError, ConnectionError) +#: Failures of the *transport*, not answers from arXiv. ``HTTPError`` is a +#: ``URLError`` subclass but is handled first, by status; ``HTTPException`` +#: covers a connection dropped mid-body (``IncompleteRead``). +TRANSIENT = (urllib.error.URLError, TimeoutError, ConnectionError, + http.client.HTTPException) _CURL_STATUS_MARKER = b"\n__PYAUTO_HTTP_STATUS__:" class ArxivAPIError(RuntimeError): - """The arXiv API could not answer a lookup reliably.""" + """The arXiv API could not answer a lookup reliably. + + Distinct from a ``None`` out of :func:`resolve_title`, which means arXiv + *did* answer and no unique title matched. Callers must never treat this as + a negative identification — in particular ``--mark-unresolved`` must not + NOTE-mark a line on the strength of it. + """ def arxiv_id(ref: str | None) -> str | None: @@ -232,13 +243,21 @@ def _request_once(url: str) -> bytes: return _curl_get(url) -def _api_query(params: dict, *, delays: tuple | None = None) -> bytes: +def _api_query(params: dict, *, delays: tuple | None = None, + sleep=time.sleep) -> bytes: """Query arXiv with retry/backoff and make transport failure explicit. September 2026 exposed two independent failure modes: arXiv can throttle shared runners with 429/406, and Fastly can reject Python's stdlib client fingerprint with a persistent 406 while curl succeeds. The latter gets one immediate alternate-transport attempt; real throttling still backs off. + + The ladder is the same shape as PyAutoMind's ``arxiv_fetch.py:_get()`` + (PyAutoMind#410), copied rather than imported: 429/406/5xx and transient + transport errors back off along `delays`, honouring ``Retry-After`` capped + at the ladder's longest step. 406 is arXiv's edge mitigation, not a + content-negotiation verdict, so it is retried like a 429. `sleep` is + injectable so the ladder can be tested without waiting it out. """ if delays is None: delays = RETRY_DELAYS @@ -269,7 +288,7 @@ def _api_query(params: dict, *, delays: tuple | None = None) -> bytes: f"retrying in {wait}s", file=sys.stderr, ) - time.sleep(wait) + sleep(wait) raise ArxivAPIError( f"arXiv API unavailable after {attempts} attempt(s): {last_error}" @@ -344,6 +363,12 @@ def resolve_title(title: str, max_results: int = 50, query: object = _api_query) -> str | None: """Look `title` up on the arXiv API and return its id, or None. + ``None`` means exactly one thing: arXiv answered, and no unique title + matched. A lookup that never got a usable answer — HTTP error, DNS/network + failure, timeout, an unparseable body — raises :class:`ArxivAPIError` + instead, so the backfill can count it as an API failure rather than as + "no unique match". + Networked — the Dashboard never calls this; only the backfill does. `query` is injected so the matching can be tested without arXiv. @@ -360,10 +385,15 @@ def resolve_title(title: str, max_results: int = 50, "start": 0, "max_results": max_results}) except ArxivAPIError: raise - except (urllib.error.URLError, TimeoutError, ConnectionError, OSError) as e: + except (*TRANSIENT, OSError) as e: # Injected query functions in tests and alternate callers may not use # _api_query(), so normalise their transport failures to the same # explicit signal. None remains reserved for a real arXiv answer that # contains no unique exact title match. raise ArxivAPIError(f"arXiv API lookup failed: {e}") from e - return match_entries(title, parse_api_entries(raw)) + try: + entries = parse_api_entries(raw) + except ET.ParseError as e: + # An edge error page or a truncated body is not an answer either. + raise ArxivAPIError(f"arXiv API returned an unparseable body: {e}") from e + return match_entries(title, entries) diff --git a/scripts/backfill_arxiv_refs.py b/scripts/backfill_arxiv_refs.py index a666dd5..2364c4d 100644 --- a/scripts/backfill_arxiv_refs.py +++ b/scripts/backfill_arxiv_refs.py @@ -19,6 +19,14 @@ re-attempted each run; that is the cost of holding no per-line failure state, and it is bounded by ``--limit``. +A *transport* failure is a different thing from a non-match and is reported as +one. ``arxiv_refs`` retries 429/406/5xx and network errors along a back-off +ladder honouring ``Retry-After``; a lookup that still gets no answer raises +``ArxivAPIError``, which this script counts as ``api_failed`` (never as +``unmatched``), stops the run on, never NOTE-marks, and turns into a non-zero +exit — so an arXiv outage goes red instead of logging "N ref-less, 0 resolved" +(2026-09-14..18, when this job ran green every night through 429s and 406s). + Run by ``.github/workflows/arxiv_refs.yml`` (a checkout has no arXiv access inside a sandboxed session; CI does). Locally: @@ -118,12 +126,21 @@ def backfill(root: Path, *, write: bool, limit: int, mark_unresolved: bool = Fal sleep=time.sleep, log=print) -> dict: """Resolve up to `limit` ref-less titles across the queue and the inbox. - With `mark_unresolved`, a line that fails its lookup is also `NOTE `-marked - so no later run asks about it again — a human-driven cleanup, never the - nightly's own decision (see :func:`mark_unresolved_line`). + With `mark_unresolved`, a line that arXiv answered for but could not + uniquely identify is also `NOTE `-marked so no later run asks about it + again — a human-driven cleanup, never the nightly's own decision (see + :func:`mark_unresolved_line`). A line whose lookup failed on *transport* + is never marked: a NOTE-mark is permanent and must only ever follow an + actual answer from arXiv. + + Stats: ``matched``/``unmatched`` count answers; ``api_failed`` counts + lookups that got no answer after the retry ladder. The first such failure + stops the run (every later lookup would hit the same dead endpoint), so + ``api_failed`` is 0 or 1 and ``unattempted`` counts the titles left behind + in that file within `limit` (later files are not opened). """ stats = {"scanned": 0, "matched": 0, "unmatched": 0, "api_failed": 0, - "marked": 0, "files": []} + "unattempted": 0, "marked": 0, "files": []} budget = limit api_down = False for name in TARGETS: @@ -134,7 +151,7 @@ def backfill(root: Path, *, write: bool, limit: int, mark_unresolved: bool = Fal todo = candidates("\n".join(lines), name) stats["scanned"] += len(todo) changed = file_marked = 0 - for idx, title in todo: + for n, (idx, title) in enumerate(todo): if budget <= 0: break budget -= 1 @@ -146,8 +163,9 @@ def backfill(root: Path, *, write: bool, limit: int, mark_unresolved: bool = Fal # --mark-unresolved must never turn an outage into permanent # NOTE markers. stats["api_failed"] += 1 + stats["unattempted"] += min(len(todo) - n - 1, budget) api_down = True - log(f" ! arXiv API failure {title[:90]} ({e})") + log(f" ! arXiv API failure {title[:90]} ({e})") break if ref: lines[idx] = apply_ref(lines[idx], ref) @@ -173,7 +191,8 @@ def backfill(root: Path, *, write: bool, limit: int, mark_unresolved: bool = Fal if write: path.write_text("\n".join(lines) + "\n") marked = f", {file_marked} NOTE-marked" if file_marked else "" - failed = f", {stats['api_failed']} API failure" if stats["api_failed"] else "" + failed = (f", {stats['api_failed']} API failure — run stopped" + if api_down else "") log(f"{name}: {len(todo)} ref-less, {changed - file_marked} resolved" f"{marked}{failed}{'' if write else ' (dry run — nothing written)'}") if api_down: @@ -199,12 +218,20 @@ def main(argv: list[str] | None = None) -> int: stats = backfill(args.root, write=args.write, limit=args.limit, mark_unresolved=args.mark_unresolved) marked = f", {stats['marked']} NOTE-marked" if stats["marked"] else "" - failed = f", {stats['api_failed']} API failure" if stats["api_failed"] else "" + failed = (f", {stats['api_failed']} arXiv API failure " + f"({stats.get('unattempted', 0)} more not attempted)" + if stats["api_failed"] else "") print(f"\n{stats['matched']} resolved, {stats['unmatched']} unresolved" f"{marked}{failed}, {stats['scanned']} ref-less lines in total.") + if stats["api_failed"]: + print("ARXIV API FAILURE: a lookup got no answer after the retry ladder; " + "the run stopped there. Unresolved counts above are answers only " + "— nothing was NOTE-marked on a failed lookup.") # A genuine no-match is normal and stays exit 0. A transport failure is a - # broken run: after _api_query has already exhausted its retry ladder we - # stop rather than misreporting the whole queue as unresolved. + # broken run: by the time it surfaces, _api_query has spent its whole retry + # ladder and the run has stopped, so transport failure is the outcome for + # every lookup after it — the dominant one. Exit non-zero so the job goes + # red; the workflow still commits whatever resolved before the failure. return 1 if stats["api_failed"] else 0 diff --git a/tests/test_arxiv_refs.py b/tests/test_arxiv_refs.py index e8b8cd6..37eed83 100644 --- a/tests/test_arxiv_refs.py +++ b/tests/test_arxiv_refs.py @@ -219,6 +219,139 @@ def rejected(*args, **kwargs): assert len(seen) == 1 +def test_witness_a_429_is_an_api_failure_not_a_non_match(tmp_path): + """Witness for the 2026-09-14..18 outage: an arXiv 429 used to come back + from resolve_title() as None — "no unique match" — so the backfill logged a + cheerful non-match, exited 0, and --mark-unresolved would have NOTE-marked + the line for good. It must surface as a transport failure instead.""" + def throttled(params): + raise urllib.error.HTTPError( + arxiv_refs.API_URL, 429, "Too Many Requests", + {"Retry-After": "30"}, None) + + with pytest.raises(arxiv_refs.ArxivAPIError): + arxiv_refs.resolve_title("A Lensed Quasar Survey", query=throttled) + + # …and through the backfill: counted as api_failed, never unmatched, + # never NOTE-marked, and the run exits non-zero. + root = _tree(tmp_path) + before = (root / "reading-queue.md").read_text() + + def resolve(title): + return arxiv_refs.resolve_title(title, query=throttled) + + stats = backfill.backfill(root, write=True, limit=10, + mark_unresolved=True, resolve=resolve, + sleep=lambda _: None, log=lambda *a: None) + assert stats["api_failed"] == 1 + assert stats["unmatched"] == 0 and stats["marked"] == 0 + assert (root / "reading-queue.md").read_text() == before + + +def test_main_exits_nonzero_on_a_real_429_run(monkeypatch, tmp_path, capsys): + root = _tree(tmp_path) + + def throttled(params): + raise _http_error(429) + + real = backfill.backfill + monkeypatch.setattr( + backfill, "backfill", + lambda *a, **kw: real( + *a, resolve=lambda t: arxiv_refs.resolve_title(t, query=throttled), + sleep=lambda _: None, **kw)) + assert backfill.main(["--root", str(root), "--write"]) == 1 + out = capsys.readouterr().out + assert "ARXIV API FAILURE" in out + assert "0 unresolved, 1 arXiv API failure (1 more not attempted)" in out + + +def _http_error(code, retry_after=None): + headers = {"Retry-After": retry_after} if retry_after else {} + return urllib.error.HTTPError(arxiv_refs.API_URL, code, "x", headers, None) + + +def _scripted(monkeypatch, *outcomes): + """Replace the single HTTP attempt with a script of errors/bodies.""" + queue = list(outcomes) + calls = [] + + def once(url): + calls.append(url) + out = queue.pop(0) if len(queue) > 1 else queue[0] + if isinstance(out, Exception): + raise out + return out + + monkeypatch.setattr(arxiv_refs, "_request_once", once) + return calls + + +def test_api_query_backs_off_on_429_and_honours_retry_after(monkeypatch): + """The retry ladder, without sleeping through it: `sleep` is injected.""" + _scripted(monkeypatch, _http_error(429, "30"), _http_error(503), b"") + slept = [] + assert arxiv_refs._api_query({"search_query": "all:x"}, + delays=(5, 15, 45), + sleep=slept.append) == b"" + # Retry-After 30 beats the 5 s step (capped at the ladder's longest, 45); + # the 503 carries none and takes the second step. + assert slept == [30, 15] + + +def test_api_query_retries_406_as_edge_mitigation(monkeypatch): + # _request_once already tried curl for this 406; it is still throttling. + _scripted(monkeypatch, _http_error(406), b"") + slept = [] + assert arxiv_refs._api_query({}, delays=(5,), sleep=slept.append) == \ + b"" + assert slept == [5] + + +def test_api_query_caps_a_hostile_retry_after(monkeypatch): + _scripted(monkeypatch, _http_error(429, "99999"), b"") + slept = [] + arxiv_refs._api_query({}, delays=(5, 15), sleep=slept.append) + assert slept == [15] + + +def test_api_query_raises_api_error_once_the_ladder_is_spent(monkeypatch): + calls = _scripted(monkeypatch, _http_error(429)) + slept = [] + with pytest.raises(arxiv_refs.ArxivAPIError): + arxiv_refs._api_query({}, delays=(1, 2, 3), sleep=slept.append) + assert len(calls) == 4 and slept == [1, 2, 3] + + +def test_api_query_does_not_retry_a_non_transient_status(monkeypatch): + calls = _scripted(monkeypatch, _http_error(400)) + slept = [] + with pytest.raises(arxiv_refs.ArxivAPIError): + arxiv_refs._api_query({}, delays=(1, 2), sleep=slept.append) + assert len(calls) == 1 and slept == [] + + +def test_api_query_retries_network_errors(monkeypatch): + _scripted(monkeypatch, urllib.error.URLError("dns"), TimeoutError("slow"), + b"") + slept = [] + assert arxiv_refs._api_query({}, delays=(1, 2), sleep=slept.append) == \ + b"" + assert slept == [1, 2] + + +def test_an_unparseable_body_is_an_api_failure_not_a_non_match(): + with pytest.raises(arxiv_refs.ArxivAPIError): + arxiv_refs.resolve_title("A Lensed Quasar Survey", + query=lambda p: b"edge error") + + +def test_none_still_means_arxiv_answered_with_no_unique_match(): + assert arxiv_refs.resolve_title( + "A Lensed Quasar Survey", + query=lambda p: _atom(("2608.1", "Something Else"))) is None + + # --- the backfill ------------------------------------------------------------- QUEUE = ("# Reading queue\n\nprose that is not a paper\n\n---\n\n" "## Strong Lensing\n"