From aae586de69fe25c8cdce5d4775691b39db1ed639 Mon Sep 17 00:00:00 2001 From: Marius Ciepluch <11855163+norandom@users.noreply.github.com> Date: Thu, 8 Oct 2026 18:54:46 +0200 Subject: [PATCH] unseen: decide phase skips a per-candidate HTTP failure too The resilient draw only covered extraction. A per-candidate transport failure in the decide/eligibility phase -- get_commit on an unresolvable fix ref raising ValueError("http_failure") (observed: ljharb/shell-quote commit 1518179) -- still aborted the whole draft, because decide only caught source.Rejected and eligibility.RepositoryNotFound. decide now catches that http_failure and returns a "decide_http_failure" skip, and the run loop feeds decide + extract per-candidate instrument failures into one systemic guard (25 with zero successes, or >200), so a single bad commit lookup skips while a broken instrument (e.g. a network outage failing every lookup) still aborts loudly. Full unseen draft suite passes (48), ruff clean. Co-Authored-By: Claude Opus 4.8 --- benchmarks/unseen/draft.py | 18 ++++++++++++++---- tests/test_unseen_draft.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/benchmarks/unseen/draft.py b/benchmarks/unseen/draft.py index 2f76b82..7da9c04 100644 --- a/benchmarks/unseen/draft.py +++ b/benchmarks/unseen/draft.py @@ -271,6 +271,12 @@ def decide(row, github, osv, used, selected_networks): return {"rejected": str(exc)} except eligibility.RepositoryNotFound: return {"rejected": "repository_or_commit_not_found"} + except ValueError as exc: + # A per-candidate transport failure (e.g. get_commit on an unresolvable fix ref) + # skips this advisory; the run loop's systemic guard catches a broken instrument. + if str(exc) == "http_failure": + return {"rejected": "decide_http_failure"} + raise def extract_repository(candidate, github, clones, fixes, seed): @@ -358,7 +364,7 @@ def run( report["advisories"] = len(rows) fixes_by_repo = advisory_index(rows, api) selected_networks, entries = set(), [] - extract_failures = 0 + instrument_failures = 0 for row in source.ordered_advisories(rows, seed): checkpoint.boundary() key = row["ghsa_id"] @@ -369,6 +375,9 @@ def run( decision = checkpoint.rows["eligibility"][key] if "rejected" in decision: rejected[decision["rejected"]] += 1 + instrument_failures += int(decision["rejected"] == "decide_http_failure") + if (instrument_failures >= 25 and not entries) or instrument_failures > 200: + raise extract.InstrumentFailure("systemic_extraction_failure") continue candidate = source.Candidate(**decision["candidate"]) # Multiple advisories for one repository can have different fix/range inputs. @@ -381,12 +390,13 @@ def run( ordinary_counts.update(result["ordinary_exclusions"]) if "rejected" in result: rejected[result["rejected"]] += 1 - extract_failures += int(result["rejected"].startswith("extract_")) + instrument_failures += int(result["rejected"].startswith("extract_")) else: selected_networks.add(decision["network"]) entries.append(result["entry"]) - # Include replayed extraction results, but never eligibility exclusions. - if (extract_failures >= 25 and not entries) or extract_failures > 200: + # Count per-candidate instrument failures (decide + extract, replay included), + # never ordinary eligibility exclusions. + if (instrument_failures >= 25 and not entries) or instrument_failures > 200: raise extract.InstrumentFailure("systemic_extraction_failure") if len(entries) == 300: break diff --git a/tests/test_unseen_draft.py b/tests/test_unseen_draft.py index 71fe270..a07b03f 100644 --- a/tests/test_unseen_draft.py +++ b/tests/test_unseen_draft.py @@ -877,3 +877,37 @@ def test_filtered_http_population_reaches_draft_floor_with_bounded_pages(tmp_pat assert report["status"] == "draft" and report["repositories"] == 300 assert report["advisories"] == 300 assert pages == [("pip", n) for n in (0, 100, 200)] + [(eco, 0) for eco in s.ECOSYSTEMS[1:]] + + +def test_decide_http_failure_skips_candidate_and_reaches_target(tmp_path, monkeypatch): + # A per-candidate transport failure in decide (e.g. get_commit on an unresolvable + # fix ref) is skipped as decide_http_failure; the draft still reaches its target. + class HTTPFailAPI(FakeAPI): + def __init__(self, count, bad): + super().__init__(count) + self.bad = bad + + def get_commit(self, name, sha): + if name in self.bad: + raise ValueError("http_failure") + return super().get_commit(name, sha) + + bad = {f"fixture/project{i:03d}" for i in range(3)} + api, used = HTTPFailAPI(303, bad), e.UsedSet() + changes = entries()[0]["changes"] + monkeypatch.setattr(d, "repository_changes", lambda *a, **kw: (changes, {}, {"bytes": 12, "files": 1})) + report = d.run(tmp_path, github=api, osv=FakeOSV(), clones=FakeClones(), used=used, seed=7, known_empty=set(used.sources)) + assert report["status"] == "draft" and report["repositories"] == 300 + assert report["rejections"].get("decide_http_failure") == 3 + + +def test_decide_http_failure_systemic_guard_aborts(tmp_path): + # Every candidate failing the commit lookup is a broken instrument, not an empty result. + class AllHTTPFailAPI(FakeAPI): + def get_commit(self, name, sha): + raise ValueError("http_failure") + + api, used = AllHTTPFailAPI(30), e.UsedSet() + report = d.run(tmp_path, github=api, osv=FakeOSV(), clones=FakeClones(), used=used, seed=7, known_empty=set(used.sources)) + assert report["status"] == "instrument_failure" and report["error_type"] == "InstrumentFailure" + assert report["rejections"].get("decide_http_failure") == 25