diff --git a/scripts/check_adjoint_prevalidation.py b/scripts/check_adjoint_prevalidation.py index 8d7774b0c3..6b04e65ea3 100644 --- a/scripts/check_adjoint_prevalidation.py +++ b/scripts/check_adjoint_prevalidation.py @@ -103,7 +103,7 @@ import subprocess import sys from dataclasses import dataclass -from typing import Any +from typing import Any, Sequence try: import gh_identity @@ -266,10 +266,21 @@ class Dossier: created_at: str = "" +API_USAGE: dict[str, int] = {"rest": 0, "graphql": 0} + + def gh_json(args: list[str]) -> Any: proc = subprocess.run( ["gh", *args], capture_output=True, text=True, encoding="utf-8" ) + # Compteur REST/GraphQL (#17315) : tout l'organe paie ici, la mesure aussi. + # GraphQL (`api graphql`) consomme le bucket partage de la flotte ; tout le + # reste (REST `api`, `pr view`, ...) paie le bucket core. Publie par + # ``--json`` (cle ``api_usage``) et sur stderr en fin d'invocation. + if len(args) >= 2 and args[0] == "api" and args[1] == "graphql": + API_USAGE["graphql"] += 1 + else: + API_USAGE["rest"] += 1 if proc.returncode != 0: raise RuntimeError(proc.stderr.strip() or "gh command failed") return json.loads(proc.stdout) @@ -608,8 +619,11 @@ def _fingerprint_payload( "threads": snapshot.get("threads") or [], } if include_checks: + # Diagnostic-only branch (#17315): reads the REST check-runs of the + # head (present in every snapshot) instead of the retired GraphQL + # rollup. Never hashed into a stamp since #16957. payload["checks"] = sorted( - snapshot.get("statusCheckRollup") or [], + snapshot.get("checkRuns") or [], key=lambda row: json.dumps( row, sort_keys=True, separators=(",", ":") ), @@ -698,30 +712,6 @@ def surfaces_fingerprint( ) -def legacy_surfaces_fingerprint( - snapshot: dict[str, Any], - comment_limit: int | None = None, - neutral_after: str | None = None, -) -> str: - """Pre-#16957 stamp algorithm: the same payload PLUS the check rollup. - - Kept so dossiers stamped before #16957 -- whose hash embedded the check - state -- remain verifiable for as long as that state is byte-identical. - Once any check concludes, a legacy stamp stops matching; recovery is one - mechanical re-stamp (--template recomputes every mechanical field, no - re-reading of surfaces), after which no check conclusion can ever expire - the dossier again. A SHA-256 over data that has since changed cannot be - re-derived, which is why zero-touch recovery of raced legacy stamps is - not offered. Drop this function when no open dossier carries a legacy - stamp. - """ - return _digest( - _fingerprint_payload( - snapshot, comment_limit, neutral_after, True, bot_forms_18637=False - ) - ) - - def pre18637_surfaces_fingerprint( snapshot: dict[str, Any], comment_limit: int | None = None, @@ -1189,20 +1179,22 @@ def validate_dossier(dossier: Dossier, snapshot: dict[str, Any]) -> list[str]: if not re.fullmatch(r"[0-9a-f]{64}", f.get("surfaces-sha256", "")): errors.append("surfaces-sha256 must be a lowercase SHA-256") # Dual acceptance (#16957): a stamp matches the post-fix fingerprint - # (discussion surfaces only) or the legacy one (which also embedded the - # check rollup). Both certify every discussion surface; the legacy digest + # (discussion surfaces only) or the pre-#18637 one (bot comments hashed + # whole). Both certify every discussion surface; the pre-#18637 digest # is strictly more fields, so accepting either weakens nothing. + # The pre-#16957 legacy algorithm (which also embedded the check rollup, + # and with it the last GraphQL consumer of the snapshot) is retired + # (#17315): its two carriers (#16950, #16891) are merged, and an eventual + # residual legacy stamp now degrades to this refusal -- fail-closed, one + # mechanical --template re-stamp recovers it. live_fingerprint = surfaces_fingerprint( snapshot, dossier.comment_index, dossier.created_at ) - legacy_fingerprint = legacy_surfaces_fingerprint( - snapshot, dossier.comment_index, dossier.created_at - ) pre18637_fingerprint = pre18637_surfaces_fingerprint( snapshot, dossier.comment_index, dossier.created_at ) if f.get("surfaces-sha256") not in { - live_fingerprint, legacy_fingerprint, pre18637_fingerprint + live_fingerprint, pre18637_fingerprint }: # #16931 defaut 3 (mesure 16928) : deux hachages opaques sont # inexploitables — la lane refabrique le dossier EN AVEUGLE. Le refus @@ -1445,24 +1437,36 @@ def build_result( return result -def review_threads(pr: int) -> list[dict[str, Any]]: - query = """ - query($owner:String!,$repo:String!,$number:Int!,$cursor:String){ - repository(owner:$owner,name:$repo){ - pullRequest(number:$number){ - reviewThreads(first:100,after:$cursor){ - nodes{ - id isResolved isOutdated path line - comments(first:100){ - totalCount - nodes{id body createdAt author{login}} - } - } - pageInfo{hasNextPage endCursor} - } - } - } - }""" +REVIEW_THREADS_BATCH = 8 + +_REVIEW_THREADS_FIELDS = ( + "reviewThreads(first:100){" + "nodes{id isResolved isOutdated path line " + "comments(first:100){totalCount " + "nodes{id body createdAt author{login}}}} " + "pageInfo{hasNextPage endCursor}}" +) + + +def _check_inline_pagination(nodes: list[dict[str, Any]]) -> None: + for thread in nodes: + inline = thread.get("comments") or {} + if inline.get("totalCount", 0) > len(inline.get("nodes") or []): + raise RuntimeError( + "inline thread has more than 100 comments; complete pagination required" + ) + + +def _review_threads_paginated(pr: int) -> list[dict[str, Any]]: + """Per-PR cursor pagination -- the fallback when one PR fills a page.""" + query = f""" + query($owner:String!,$repo:String!,$number:Int!,$cursor:String){{ + repository(owner:$owner,name:$repo){{ + pullRequest(number:$number){{ + {_REVIEW_THREADS_FIELDS} + }} + }} + }}""" owner, repo = REPO.split("/", 1) cursor: str | None = None threads: list[dict[str, Any]] = [] @@ -1477,12 +1481,7 @@ def review_threads(pr: int) -> list[dict[str, Any]]: data = gh_json(args) connection = data["data"]["repository"]["pullRequest"]["reviewThreads"] nodes = connection.get("nodes") or [] - for thread in nodes: - inline = thread.get("comments") or {} - if inline.get("totalCount", 0) > len(inline.get("nodes") or []): - raise RuntimeError( - "inline thread has more than 100 comments; complete pagination required" - ) + _check_inline_pagination(nodes) threads.extend(nodes) page = connection["pageInfo"] if not page["hasNextPage"]: @@ -1490,6 +1489,56 @@ def review_threads(pr: int) -> list[dict[str, Any]]: cursor = page["endCursor"] +def review_threads_bulk(prs: Sequence[int]) -> dict[int, list[dict[str, Any]]]: + """Review threads of N pull requests in ONE GraphQL query per batch. + + #17315: mass passes (gate sweeps, B.0 probes over the fleet) paid one + paginated query per pull request for the third B.0 surface; aliases + collapse N pulls into a single operation per batch, ~N/8 times fewer. + A pull request whose threads overflow the first page (rare) falls back + to per-PR pagination for that PR alone. + """ + results: dict[int, list[dict[str, Any]]] = {} + owner, repo = REPO.split("/", 1) + remaining = list(dict.fromkeys(prs)) + while remaining: + batch = remaining[:REVIEW_THREADS_BATCH] + del remaining[:REVIEW_THREADS_BATCH] + aliases = [f"p{i}" for i in range(len(batch))] + defs = "".join(f",${alias}:Int!" for alias in aliases) + body = "".join( + f"{alias}: repository(owner:$owner,name:$repo)" + f"{{pullRequest(number:${alias}){{{_REVIEW_THREADS_FIELDS}}}}}" + for alias in aliases + ) + query = f"query($owner:String!,$repo:String!{defs}){{{body}}}" + args = [ + "api", "graphql", "-f", f"query={query}", + "-F", f"owner={owner}", "-F", f"repo={repo}", + ] + for alias, pr in zip(aliases, batch): + args.extend(["-F", f"{alias}={pr}"]) + data = gh_json(args) + for alias, pr in zip(aliases, batch): + # L'alias nomme le `repository`; la connexion vit sous `pullRequest`. + # Mesure 2026-10-08 : lire `[alias]["reviewThreads"]` leve un KeyError + # -- le format de reponse se verifie sur un appel LIVE, un fixture + # ecrit depuis la meme lecture reproduit l'erreur a l'identique. + connection = data["data"][alias]["pullRequest"]["reviewThreads"] + nodes = connection.get("nodes") or [] + _check_inline_pagination(nodes) + if connection["pageInfo"]["hasNextPage"]: + results[pr] = _review_threads_paginated(pr) + else: + results[pr] = nodes + return results + + +def review_threads(pr: int) -> list[dict[str, Any]]: + """Single-PR convenience over the aliased bulk fetch: one operation.""" + return review_threads_bulk([pr])[pr] + + def _issue_comments(pr: int) -> list[dict[str, Any]]: rows = gh_json([ "api", f"repos/{REPO}/issues/{pr}/comments", "--paginate", @@ -1549,16 +1598,17 @@ def _head_check_runs(head_sha: str) -> list[dict[str, Any]]: page += 1 -def _pr_metadata(pr: int, *, with_rollup: bool) -> dict[str, Any]: - # Scalar fields come from REST (`repos/.../pulls/N`) so the shared GraphQL - # quota only pays for the check rollup below. Keys keep the exact shape - # `gh pr view --json` produced, so fingerprints and the identity bracket - # stay byte-compatible with dossiers stamped before this change. +def _pr_metadata(pr: int) -> dict[str, Any]: + # Scalar fields come from REST (`repos/.../pulls/N`) -- the whole snapshot + # bracket no longer touches GraphQL (#17315): the check rollup it used to + # carry served only the retired legacy fingerprint. Keys keep the exact + # shape `gh pr view --json` produced, so fingerprints and the identity + # bracket stay byte-compatible with dossiers stamped before this change. row = gh_json(["api", f"repos/{REPO}/pulls/{pr}"]) if not isinstance(row, dict): raise RuntimeError("pull request response is not an object") state = row.get("state") or "" - data: dict[str, Any] = { + return { "number": row.get("number"), "title": row.get("title"), "body": row.get("body") or "", @@ -1578,46 +1628,34 @@ def _pr_metadata(pr: int, *, with_rollup: bool) -> dict[str, Any]: "additions": row.get("additions"), "deletions": row.get("deletions"), } - if with_rollup: - # The check rollup has no REST equivalent, so it stays on GraphQL - # as a single-field query instead of the former twelve-field one. - rollup = gh_json([ - "pr", "view", str(pr), "--repo", REPO, - "--json", "statusCheckRollup", - ]) - if not isinstance(rollup, dict): - raise RuntimeError("pull request response is not an object") - data["statusCheckRollup"] = rollup.get("statusCheckRollup") - return data def _metadata_identity(data: dict[str, Any]) -> str: - normalized = dict(data) - normalized["statusCheckRollup"] = sorted( - data.get("statusCheckRollup") or [], - key=lambda row: json.dumps(row, sort_keys=True, separators=(",", ":")), - ) return json.dumps( - normalized, ensure_ascii=False, sort_keys=True, separators=(",", ":") + data, ensure_ascii=False, sort_keys=True, separators=(",", ":") ) def load_snapshot(pr: int) -> dict[str, Any]: - before = _pr_metadata(pr, with_rollup=True) + before = _pr_metadata(pr) snapshot = dict(before) snapshot["comments"] = _issue_comments(pr) snapshot["reviews"] = _reviews(pr) snapshot["threads"] = review_threads(pr) - # Fetched inside the before/after bracket: a check concluding during the - # read bumps updatedAt and aborts the snapshot (transient UNKNOWN, the - # caller retries), so the claim verification below never reads a state - # that was already stale when captured. The B.0 probe (`probe_b0`) is NOT - # in this bracket: it runs after, and only on a READY dossier. A remark - # posted between the snapshot and the probe therefore makes the organ - # contradict a `b0: clear` claim -- a conservative refusal, which a rerun - # names as a changed discussion surface. + # Fetched inside the before/after bracket, keyed on the head read BEFORE + # it: a push landing during the read moves headRefOid, the second metadata + # read disagrees, and the snapshot is refused (transient UNKNOWN, the + # caller retries) rather than pairing the old head's check-runs with the + # new one. Since #17315 the bracket covers PR scalar fields only -- the + # rollup that used to carry check state is gone, so a check changing + # conclusion mid-read is not itself a bracket event; the claim + # verification below compares against the live check-runs. The B.0 probe + # (`probe_b0`) is NOT in this bracket: it runs after, and only on a READY + # dossier. A remark posted between the snapshot and the probe therefore + # makes the organ contradict a `b0: clear` claim -- a conservative + # refusal, which a rerun names as a changed discussion surface. snapshot["checkRuns"] = _head_check_runs(snapshot["headRefOid"]) - after = _pr_metadata(pr, with_rollup=True) + after = _pr_metadata(pr) if _metadata_identity(before) != _metadata_identity(after): raise RuntimeError("pull request changed while prevalidation snapshot was read") return snapshot @@ -1945,6 +1983,9 @@ def main() -> int: result["ready"] = False result["verdict"] = "FROZEN" result["frozen"] = frozen_reason + # #17315 : le cout de l'invocation est publie avec le verdict -- le + # compteur distingue les deux buckets (REST core vs GraphQL partage). + result["api_usage"] = dict(API_USAGE) if args.json: print(json.dumps(result, ensure_ascii=False)) elif frozen_reason is not None: @@ -1975,12 +2016,21 @@ def main() -> int: for error in errors: print(f" - {error}") if frozen_reason is not None: - return EXIT_BLOCKED_WITH_SUBSTANCE - if ready: - return EXIT_READY - if verdict == VERDICT_BLOCKED: - return EXIT_BLOCKED_WITH_SUBSTANCE - return EXIT_NO_DOSSIER + code = EXIT_BLOCKED_WITH_SUBSTANCE + elif ready: + code = EXIT_READY + elif verdict == VERDICT_BLOCKED: + code = EXIT_BLOCKED_WITH_SUBSTANCE + else: + code = EXIT_NO_DOSSIER + # #17315 : chiffres publies sur stderr (stdout reste le verdict parse par + # les appelants) -- une lane secretaire peut agreger le cout par invocation. + print( + f"API usage: {API_USAGE['rest']} REST, " + f"{API_USAGE['graphql']} GraphQL operation(s)", + file=sys.stderr, + ) + return code if __name__ == "__main__": diff --git a/scripts/tests/test_check_adjoint_prevalidation.py b/scripts/tests/test_check_adjoint_prevalidation.py index d289ae3ad6..e9017f2720 100644 --- a/scripts/tests/test_check_adjoint_prevalidation.py +++ b/scripts/tests/test_check_adjoint_prevalidation.py @@ -40,7 +40,6 @@ def _base_snapshot() -> dict: "comments": [_comment("ordinary earlier comment")], "reviews": [{"state": "COMMENTED"}, {"state": "APPROVED"}], "threads": [{"isResolved": True}], - "statusCheckRollup": [{"name": "PR gate", "conclusion": "SUCCESS"}], "checkRuns": [ { "id": 1, @@ -646,18 +645,11 @@ def test_patched_restamp_can_never_match_but_a_new_comment_does(): assert ready -def test_metadata_identity_ignores_only_check_order(): - first = { - "number": 123, - "updatedAt": "2026-09-16T19:00:00Z", - "statusCheckRollup": [ - {"name": "A", "conclusion": "SUCCESS"}, - {"name": "B", "conclusion": "SUCCESS"}, - ], - } - reordered = dict(first) - reordered["statusCheckRollup"] = list(reversed(first["statusCheckRollup"])) - assert mod._metadata_identity(first) == mod._metadata_identity(reordered) +def test_metadata_identity_is_exact(): + """#17315: the rollup is out of the snapshot, so the identity is a plain + canonical rendering -- equal dicts match, any field move diverges.""" + first = {"number": 123, "updatedAt": "2026-09-16T19:00:00Z"} + assert mod._metadata_identity(first) == mod._metadata_identity(dict(first)) changed = dict(first) changed["updatedAt"] = "2026-09-16T19:00:01Z" @@ -687,7 +679,7 @@ def _bracket_reads(monkeypatch, first, second=None): } ] - def fake_pr_metadata(pr, *, with_rollup): + def fake_pr_metadata(pr): calls["metadata"] += 1 factory = first if calls["metadata"] == 1 else (second or first) return factory() @@ -726,23 +718,23 @@ def test_load_snapshot_reads_metadata_twice_and_takes_checks_from_rest(monkeypat assert snapshot["threads"] == [{"thread": 1}] -def test_load_snapshot_aborts_when_a_check_concludes_during_the_read(monkeypatch): - """#17390 acceptance 2: the guard must go red, not absolve. +def test_load_snapshot_aborts_when_the_head_moves_during_the_read(monkeypatch): + """#17390 acceptance 2, re-based on the REST bracket (#17315). - A check moving from in-progress to a conclusion between the two metadata - reads makes the snapshot born of an already-stale state: the gate refuses - it (transient UNKNOWN, the caller retries) instead of certifying it. + The check-runs are keyed on `headRefOid` and fetched inside the bracket, so + a push landing between the two metadata reads must abort: accepting the + snapshot would pair the OLD head's check-runs with the NEW head, and the + claim verification would then read a state that never existed together. """ - in_progress = _metadata_payload( - statusCheckRollup=[{"name": "PR gate", "status": "IN_PROGRESS", "conclusion": None}] - ) - concluded = _metadata_payload( - statusCheckRollup=[{"name": "PR gate", "status": "COMPLETED", "conclusion": "SUCCESS"}] - ) - _bracket_reads(monkeypatch, lambda: in_progress, lambda: concluded) + first = _metadata_payload(headRefOid=HEAD) + pushed = _metadata_payload(headRefOid="f" * 40) + calls, _runs = _bracket_reads(monkeypatch, lambda: first, lambda: pushed) with pytest.raises(RuntimeError, match="changed while prevalidation snapshot was read"): mod.load_snapshot(123) + # The check-runs were fetched for the head read BEFORE the push -- which is + # exactly why the mismatch is refused instead of silently accepted. + assert calls["check_runs_head"] == HEAD def test_load_snapshot_aborts_when_any_surface_moves_during_the_read(monkeypatch): @@ -1151,13 +1143,11 @@ def test_check_completing_green_after_dossier_does_not_expire_it(): assert verdict == mod.VERDICT_READY, errors -def test_green_check_may_also_move_the_rollup_after_the_stamp(): - """The rollup is neither hashed (new stamps) nor read for the claim: only - the per-name latest-wins verdicts of the head's check-runs are.""" +def test_green_check_landing_after_the_stamp_keeps_ready(): + """Only the per-name latest-wins verdicts of the head's check-runs matter: + a green run appearing after the stamp neither expires the dossier nor + contradicts its `latest-wins-green` claim.""" snapshot = _snapshot(_body()) - snapshot["statusCheckRollup"].append( - {"name": "perimeter review guard (#11268)", "conclusion": "SUCCESS"} - ) snapshot["checkRuns"].append( {"id": 2, "name": "perimeter review guard (#11268)", "status": "completed", "conclusion": "success", @@ -1349,41 +1339,32 @@ def test_blocked_dossier_is_not_refuted_by_a_red_check(): assert verdict == mod.VERDICT_BLOCKED, errors -def test_legacy_stamp_still_accepted_while_checks_unchanged(): - """Backward acceptance: pre-#16957 stamps hashed the rollup as well; both - digests are valid certificates of the discussion surfaces.""" - legacy = mod.legacy_surfaces_fingerprint(_base_snapshot()) - snapshot = _snapshot(_body(**{"surfaces-sha256": legacy})) - verdict, errors = mod.evaluate(snapshot) - assert verdict == mod.VERDICT_READY, errors - +def test_retired_legacy_stamp_needs_a_mechanical_restamp(): + """#17315: the pre-#16957 fingerprint is retired with its last carrier. -def test_legacy_stamp_whose_checks_moved_needs_a_mechanical_restamp(): - """A legacy stamp whose checks moved matches NEITHER digest: the hash - embedded data that has since changed, and a SHA-256 over changed data - cannot be re-derived. The refusal names the recovery (--template), and the - re-stamped dossier can never again be expired by a check conclusion.""" - snapshot = _snapshot(_body(**{ - "surfaces-sha256": mod.legacy_surfaces_fingerprint(_base_snapshot()) - })) - snapshot["statusCheckRollup"][0]["conclusion"] = "PENDING" + Both carriers merged (#16950/#16891), so a dossier still stamped with it + matches neither live digest. The organ fails CLOSED and names the single + recovery: a mechanical `--template` re-stamp. + """ + assert not hasattr(mod, "legacy_surfaces_fingerprint") + # A stamp matching neither accepted digest is the shape of a retired-legacy + # dossier: fail CLOSED, with the one gesture that can recover it named. + snapshot = _snapshot(_body(**{"surfaces-sha256": "a" * 64})) errors = _errors(snapshot) assert any( - "discussion surfaces changed" in error - and "--template re-stamp" in error + "discussion surfaces changed" in error and "--template re-stamp" in error for error in errors ) def test_surfaces_fingerprint_is_insensitive_to_check_state(): - """The new digest must not move when only checks move -- that is the race - being closed. The legacy digest still does, which is why it is legacy.""" + """The live digest must not move when only checks move -- that is the race + being closed. Nothing else in the module hashes check state either.""" base = _base_snapshot() moved = _base_snapshot() moved["checkRuns"][0]["conclusion"] = "failure" - moved["statusCheckRollup"] = [{"name": "PR gate", "conclusion": "FAILURE"}] assert mod.surfaces_fingerprint(base) == mod.surfaces_fingerprint(moved) - assert mod.legacy_surfaces_fingerprint(base) != mod.legacy_surfaces_fingerprint(moved) + assert mod.pre18637_surfaces_fingerprint(base) == mod.pre18637_surfaces_fingerprint(moved) def test_template_renders_the_emitting_lane_not_a_borrowed_name(): @@ -2647,3 +2628,191 @@ def test_render_emitted_dossier_blocked_at_same_head_then_changed_head(monkeypat block, verdict, _reasons = mod.render_emitted_dossier(snapshot, mod.ADJOINT_LANE) assert verdict == mod.VERDICT_READY assert "supersedes" not in block + + +# --- #17315 : le cout de l'invocation est mesure et publie ------------------- +# +# Le bucket GraphQL est partage par toute la flotte ; le bucket REST (core) est +# par machine. L'organe publie les deux pour qu'une lane secretaire puisse +# agreger le cout d'une campagne sans instrumenter le reseau. + + +def _reset_api_usage() -> None: + mod.API_USAGE["rest"] = 0 + mod.API_USAGE["graphql"] = 0 + + +class _Completed: + def __init__(self, stdout: str) -> None: + self.returncode = 0 + self.stdout = stdout + self.stderr = "" + + +def test_api_usage_counter_separates_the_two_buckets(monkeypatch): + """`gh api graphql` paie le bucket partage, tout le reste le bucket core.""" + monkeypatch.setattr( + mod.subprocess, "run", lambda *a, **k: _Completed('{"ok": true}') + ) + _reset_api_usage() + + mod.gh_json(["api", "graphql", "-f", "query={...}"]) + mod.gh_json(["api", "repos/o/r/pulls/1"]) + mod.gh_json(["api", "repos/o/r/commits/deadbeef/check-runs"]) + mod.gh_json(["pr", "view", "1", "--json", "state"]) + + assert mod.API_USAGE == {"rest": 3, "graphql": 1} + + +def test_api_usage_is_published_in_json_and_on_stderr(monkeypatch, capsys): + """Le cout accompagne le verdict : cle `api_usage` en --json, ligne stderr + sinon -- stdout reste le verdict que les appelants parsent.""" + _reset_api_usage() + rc = _run_main(monkeypatch, _snapshot(_body()), "--json") + captured = capsys.readouterr() + + assert rc == mod.EXIT_READY + payload = json.loads(captured.out) + assert payload["api_usage"] == {"rest": 0, "graphql": 0} + assert "API usage: 0 REST, 0 GraphQL operation(s)" in captured.err + + +# --- #17315 : les reviewThreads passent en une operation par lot ------------- + + +def _bulk_gh_json(calls: list[list[str]], payload_for): + def fake(args: list[str]): + calls.append(list(args)) + assert args[0] == "api" and args[1] == "graphql" + # Les alias demandes sont les `-F pN=` de la requete. + prs = [int(a.split("=", 1)[1]) for a in args if a.startswith("p") and "=" in a] + query = next(a for a in args if a.startswith("query=")) + requested = sorted(prs) if "repository(owner:$owner" in query else [] + return payload_for(requested, args) + + return fake + + +def _threads_payload(prs, *, overflow=(), unresolved=0): + data = {} + for index, pr in enumerate(prs): + nodes = [ + {"id": f"t{pr}-{n}", "isResolved": n >= unresolved, "path": "a.py", + "line": n, "comments": {"totalCount": 1, "nodes": [{"id": "c"}]}} + for n in range(1, 2 + (1 if pr in overflow else 0)) + ] + data[f"p{index}"] = { + "pullRequest": { + "reviewThreads": { + "nodes": nodes, + "pageInfo": {"hasNextPage": pr in overflow, "endCursor": "cur"}, + } + } + } + return {"data": data} + + +def test_review_threads_bulk_collapses_n_prs_into_one_operation(monkeypatch): + """Le geste mesure : N PRs = 1 operation GraphQL, pas N.""" + calls: list[list[str]] = [] + monkeypatch.setattr( + mod, "gh_json", _bulk_gh_json(calls, lambda prs, args: _threads_payload(prs)) + ) + + result = mod.review_threads_bulk([11, 12, 13, 14]) + + assert len(calls) == 1 + assert sorted(result) == [11, 12, 13, 14] + # Chaque PR recoit SES threads, pas ceux de l'alias voisin. + assert [t["id"] for t in result[13]] == ["t13-1"] + + +def test_review_threads_bulk_chunks_above_the_batch_size(monkeypatch): + calls: list[list[str]] = [] + monkeypatch.setattr( + mod, "gh_json", _bulk_gh_json(calls, lambda prs, args: _threads_payload(prs)) + ) + + result = mod.review_threads_bulk(list(range(1, 10))) + + assert len(calls) == 2, "9 PRs a batch=8 = 2 operations" + assert sorted(result) == list(range(1, 10)) + + +def test_review_threads_bulk_deduplicates_and_keeps_order(monkeypatch): + calls: list[list[str]] = [] + monkeypatch.setattr( + mod, "gh_json", _bulk_gh_json(calls, lambda prs, args: _threads_payload(prs)) + ) + + result = mod.review_threads_bulk([7, 7, 7]) + + assert len(calls) == 1 + assert sorted(result) == [7] + + +def test_review_threads_bulk_falls_back_per_pr_on_overflow(monkeypatch): + """Une PR dont les threads debordent la premiere page repart en pagination + par PR -- pour ELLE seule ; les autres restent dans l'operation du lot.""" + calls: list[list[str]] = [] + pages = {13: 0} + + def fake(args: list[str]): + calls.append(list(args)) + if "query($owner:String!,$repo:String!,$number:Int!" in " ".join(args): + pages[13] += 1 + if pages[13] == 1: + return { + "data": {"repository": {"pullRequest": {"reviewThreads": { + "nodes": [{"id": "t13-page1", "isResolved": True, + "comments": {"totalCount": 1, "nodes": [{}]}}], + "pageInfo": {"hasNextPage": True, "endCursor": "c1"}, + }}}} + } + return { + "data": {"repository": {"pullRequest": {"reviewThreads": { + "nodes": [{"id": "t13-page2", "isResolved": True, + "comments": {"totalCount": 1, "nodes": [{}]}}], + "pageInfo": {"hasNextPage": False, "endCursor": None}, + }}}} + } + prs = [int(a.split("=", 1)[1]) for a in args if a.startswith("p") and "=" in a] + return _threads_payload(prs, overflow={13}) + + monkeypatch.setattr(mod, "gh_json", fake) + + result = mod.review_threads_bulk([12, 13]) + + assert [t["id"] for t in result[13]] == ["t13-page1", "t13-page2"] + assert [t["id"] for t in result[12]] == ["t12-1"] + assert len(calls) == 3, "1 lot + 2 pages pour la seule PR 13" + + +def test_review_threads_bulk_preserves_the_inline_pagination_refusal(monkeypatch): + """Le garde-fou >100 commentaires inline survit au batching : sans lui une + page tronquee se lirait comme un thread complet.""" + + def fake(args: list[str]): + return {"data": {"p0": {"pullRequest": {"reviewThreads": { + "nodes": [{"id": "t", "isResolved": True, "comments": { + "totalCount": 120, "nodes": [{"id": "c"}], + }}], + "pageInfo": {"hasNextPage": False, "endCursor": None}, + }}}}} + + monkeypatch.setattr(mod, "gh_json", fake) + + with pytest.raises(RuntimeError, match="more than 100 comments"): + mod.review_threads_bulk([42]) + + +def test_review_threads_single_pr_is_still_one_operation(monkeypatch): + calls: list[list[str]] = [] + monkeypatch.setattr( + mod, "gh_json", _bulk_gh_json(calls, lambda prs, args: _threads_payload(prs)) + ) + + threads = mod.review_threads(9) + + assert len(calls) == 1 + assert [t["id"] for t in threads] == ["t9-1"]