Coverage for src/keel/swarm_review.py: 100%
377 statements
« prev ^ index » next coverage.py v7.16.2, created at 2026-10-02 20:26 +0000
« prev ^ index » next coverage.py v7.16.2, created at 2026-10-02 20:26 +0000
1"""What ``keel swarm-review`` decides about a cluster's pull request (#1423).
3The owner's decision on #1423: keel dispatches each cluster pull request's reviewer seats
4itself and posts their verdicts with ``keel review``, pinned to the pull request's head, so
5``swarm-run --live`` -> ``swarm-review --live`` -> ``swarm-land --live`` works without a host
6agent. It is its own opt-in step; neither ``swarm-run`` nor ``swarm-land`` calls it.
8This module holds every *decision* in that step, and nothing else:
10- **Consent.** :func:`consent_refusal` reads the operator's contract over
11 :data:`REVIEW_SIDE_EFFECTS` — a checkout per seat (``filesystem``, ``git``) and the posted
12 verdicts (``github``) — and accepts only an approved one: keel dispatches the seats and
13 posts their verdicts itself, so no host agent is there to approve them.
14- **Who reviews.** The cluster's planned reviewer seats — the ones
15 :func:`keel.team.resolve_assignment` resolved for it, which ``swarm-plan`` prints — each
16 planned exactly as ``keel delegate run --provider <seat> --role review`` would plan it
17 (:func:`plan_review_seat`). A seat keel cannot run read-only is refused, and so is one from
18 the implementer's own vendor; :func:`cluster_refusal` then refuses the cluster when what is
19 left cannot meet the tier's verdict count or ``require_distinct_vendors``.
20- **What a seat is told.** :func:`render_review_brief`: ``/keel:ship`` s7's briefing — the
21 focus slice, the refute-not-approve stance, no cross-reading, the head pinned — with the
22 issue text and the diff keel read, and the one JSON verdict it must answer with.
23- **How a seat's answer is read.** :func:`read_seat_verdict`: the verdict word first, the
24 evidence gate's own way. A seat that expressed a rejection is never discarded — it is posted
25 as ``REQUEST_CHANGES`` with whatever of its answer is usable. Only an approval must pass the
26 loader ``keel review --reviews`` uses (:func:`keel.review.parse_reviews`) and the evidence
27 gate's substance rule; an approval that does not, and an answer with no readable verdict, is
28 a failed seat, never an approval.
29- **Whether anything is posted.** :func:`posting_decision` / :func:`posted_items`. The
30 evidence gate reads each verdict's ``Verdict:`` line (#1426), so a rejection is always
31 posted and holds ``keel merge`` with ``review-verdict-not-approved``. A failed seat holds
32 the cluster's approvals (fail closed: it may have been about to reject), so beside one only
33 the rejections are posted; a moved head, or a seat that changed the repository's git setup,
34 posts nothing for the cluster.
36Pure and deterministic: no subprocess, no filesystem, no clock. The runtime
37(:mod:`keel.swarm_review_runtime`) checks the head out, runs the seats and posts.
38"""
40from __future__ import annotations
42import json
43import re
44from collections.abc import Callable, Mapping, Sequence
45from dataclasses import dataclass, field, replace
46from typing import Any, NamedTuple
48from . import artifacts, consent, delegate, evidence, review, swarm_worker, team
49from . import findings as fnd
50from . import providers as providers_mod
52#: Every mutation a live ``swarm-review`` performs: a checkout of the head per seat, and the
53#: verdicts posted on the pull request. The operator's consent is asked over exactly these.
54REVIEW_SIDE_EFFECTS = ("git_worktree", "comments")
56#: The role every seat runs in: :data:`keel.delegate.READ_ONLY_ROLES`.
57REVIEW_ROLE = "review"
59#: The verdict keel posts for a seat whose answer approves, and for one that requests
60#: changes. Whether an answer approves is the evidence gate's own reading of it
61#: (:func:`keel.evidence.verdict_approves`, :data:`keel.evidence.APPROVING_VERDICTS`), so
62#: swarm-review and ``keel merge`` cannot disagree about it.
63APPROVE = "APPROVE"
64REQUEST_CHANGES = "REQUEST_CHANGES"
65#: A seat that answered nothing keel can read: never an approval.
66FAILED = "failed"
68#: What became of one cluster. :data:`POSTED`: every seat approved and the approvals were
69#: posted. :data:`POSTED_CHANGES_REQUESTED`: the verdicts were posted and at least one requests
70#: changes, so ``keel merge`` holds the pull request on it.
71POSTED = "posted"
72POSTED_CHANGES_REQUESTED = "posted-changes-requested"
73PLANNED = "planned"
74HELD = "held"
75REFUSED = "refused"
76SKIPPED = "skipped"
77MERGED = "already-merged"
78ERROR = "failed"
79#: The outcomes a clean run ends with: approvals posted (live), a plan printed (dry run), or
80#: nothing left to review because the pull request has merged. A posted change request is
81#: not clean: the pull request will not land until it is addressed.
82CLEAN_STATUSES = (POSTED, PLANNED, MERGED)
84#: How much of the diff a brief carries. Past it the brief says the diff was cut, and the
85#: seat reads the rest in its checkout of the head.
86MAX_DIFF_CHARS = 200_000
88#: ``/keel:ship`` s7's reviewer stance (``src/keel/adapters/commands/ship.md``), all four
89#: together: the first without the rest is worse than neither.
90REVIEW_STANCE = (
91 (
92 "**Refute it.** Default to the position that the change is wrong and concede only "
93 "when the code forces you to. The author already made the case for it; nobody has "
94 "made the case against it."
95 ),
96 (
97 "**A finding you cannot demonstrate is not a finding.** Prefer a reproduction — a "
98 "failing input, a trace through the code to the line — over an assertion."
99 ),
100 (
101 "**Finish the trace.** Follow a defect from where you noticed it to where it "
102 "actually lands: a wrong value on a screen and a wrong value written to a record "
103 "are the same bug with very different severities."
104 ),
105 (
106 '**"I checked X, Y and Z and found nothing" is a complete review.** Say what you '
107 "checked. A manufactured finding is a failure of the review, not a strict one."
108 ),
109)
112def required_scopes() -> tuple[str, ...]:
113 """The consent scopes a live ``swarm-review`` needs: those of :data:`REVIEW_SIDE_EFFECTS`."""
114 return consent.side_effect_scopes(REVIEW_SIDE_EFFECTS)
117def consent_refusal(contract: Mapping[str, Any]) -> str:
118 """Why a live ``swarm-review`` may not start under ``contract``; ``""`` when it may.
120 Asked before anything is read or run. A missing scope refuses, and so does consent left
121 to a host agent (``consent_mode: agent``): keel runs the seats and posts their verdicts
122 itself, so the operator approves the scopes explicitly.
123 """
124 ok, message = consent.assert_operator_consent(dict(contract))
125 if not ok:
126 return message
127 status = contract.get("status")
128 if status != "approved":
129 return (
130 f"operator consent is {status!r}, which approves nothing keel can act on: "
131 "swarm-review --live dispatches its reviewer seats and posts their verdicts "
132 f"itself, so the operator approves the scopes (--approve-scope "
133 f"{','.join(required_scopes())} --operator NAME, or KEEL_APPROVE_SCOPE with "
134 "KEEL_OPERATOR under consent_mode: standing)"
135 )
136 return ""
139class PullRequestFacts(NamedTuple):
140 """What keel read from a cluster's pull request before any seat runs."""
142 head_sha: str
143 title: str
144 #: The tier ``keel review`` resolves from the pull request's own diff.
145 tier: int | None
146 #: The review contract ``keel review`` resolves for it — the count it will refuse to
147 #: under-post, ``require_distinct_vendors``, the focus slices and the project's additions.
148 contract: Mapping[str, Any]
149 #: The unified diff at the head; empty in a dry run, which briefs no seat.
150 diff: str = ""
153def _reviewers(contract: Mapping[str, Any]) -> Mapping[str, Any]:
154 block = contract.get("reviewers")
155 return block if isinstance(block, Mapping) else {}
158def required_count(contract: Mapping[str, Any]) -> int:
159 """The verdicts ``keel review`` requires for the pull request (``tier requires at least``)."""
160 count = _reviewers(contract).get("count")
161 return count if isinstance(count, int) and not isinstance(count, bool) else 0
164def distinct_vendors_required(contract: Mapping[str, Any]) -> bool:
165 return _reviewers(contract).get("require_distinct_vendors") is True
168def _strings(value: Any) -> tuple[str, ...]:
169 if not isinstance(value, list):
170 return ()
171 return tuple(v.strip() for v in value if isinstance(v, str) and v.strip())
174def focus_for(contract: Mapping[str, Any], slot: str, index: int) -> tuple[str, ...]:
175 """The focus slice ``/keel:ship`` s7 gives the reviewer in ``slot``.
177 The contract's ``reviewers.focuses`` names one slice per slot, merging dimensions when
178 the count drops (they merge, never drop). A slot it does not name — a bench larger
179 than the count — takes the slice at its position, and past the last one, every
180 dimension.
181 """
182 focuses = [f for f in _reviewers(contract).get("focuses") or () if isinstance(f, Mapping)]
183 for entry in focuses:
184 if entry.get("slot") == slot:
185 return _strings(entry.get("focus"))
186 if index < len(focuses):
187 return _strings(focuses[index].get("focus"))
188 return tuple(dim for entry in focuses for dim in _strings(entry.get("focus")))
191def restaffed(persisted: Mapping[str, Any] | None, resolved: Mapping[str, Any]) -> dict[str, Any]:
192 """``resolved``'s review bench, with the implementer the run actually dispatched.
194 ``--reviewers`` / ``--review-delegate`` re-resolve a cluster's assignment through the
195 resolver ``swarm-plan`` used (:func:`keel.swarm.resolve_cluster_assignment`). Only the
196 bench may change: the implementer is the seat that wrote the pull request, which is the
197 one a reviewer's vendor is compared with.
198 """
199 staffed = dict(resolved)
200 if persisted and isinstance(persisted.get("implementer"), Mapping):
201 staffed["implementer"] = dict(persisted["implementer"])
202 return staffed
205def restaff_plan(plan: Any, resolve: Callable[[Any], Mapping[str, Any] | None]) -> Any:
206 """``plan`` with each cluster's bench re-resolved by ``resolve`` (:func:`restaffed`).
208 ``resolve`` answers ``None`` for a cluster it cannot re-resolve (a plan with no
209 difficulty recorded for it), which keeps the persisted bench.
210 """
211 waves = []
212 for wave in plan.waves:
213 clusters = []
214 for cluster in wave.clusters:
215 resolved = resolve(cluster)
216 if resolved is not None:
217 cluster = replace(cluster, assignment=restaffed(cluster.assignment, resolved))
218 clusters.append(cluster)
219 waves.append(replace(wave, clusters=tuple(clusters)))
220 return replace(plan, waves=tuple(waves))
223def seat_token(seat: Mapping[str, Any]) -> str:
224 """``provider`` or ``provider:model`` — the ``--provider`` token ``keel delegate run`` takes."""
225 model = seat.get("model")
226 return str(seat.get("provider")) + (f":{model}" if model else "")
229def seat_vendor(
230 seat: Mapping[str, Any] | None,
231 *,
232 config: Any,
233 registry: providers_mod.Registry | None,
234 host_agent: str,
235) -> str:
236 """The vendor attribution names for a seat — the implementer's, here.
238 A provider seat resolves as ``keel delegate run`` resolves it (a profile or registry
239 entry's ``vendor_label`` included). A host subagent runs in the host agent. ``""`` when
240 the cluster records no seat at all.
241 """
242 if not seat:
243 return ""
244 if seat.get("kind") != "provider":
245 return host_agent
246 try:
247 resolution = delegate.resolve_provider(config, registry, seat_token(seat))
248 except delegate.DelegateError:
249 return str(seat.get("name") or seat.get("provider"))
250 return resolution.provider.label_vendor()
253@dataclass(frozen=True)
254class ReviewSeat:
255 """One reviewer seat of a cluster, as keel would dispatch it, or why it would not."""
257 slot: str
258 #: The ``reviewer`` its verdict carries — stable per slot and vendor, so a second run
259 #: on the same head edits the same comment rather than adding a verdict.
260 reviewer: str
261 #: The seat's ``--provider`` token.
262 provider: str
263 #: Where the seat came from (``team.review``, ``flag:--review-delegate`` …).
264 source: str
265 plan: delegate.RunPlan | None = None
266 refusal: str = ""
268 @property
269 def eligible(self) -> bool:
270 return self.plan is not None and not self.refusal
272 @property
273 def vendor(self) -> str | None:
274 return None if self.plan is None else self.plan.vendor
276 def to_dict(self) -> dict[str, Any]:
277 plan = self.plan
278 return {
279 "slot": self.slot,
280 "reviewer": self.reviewer,
281 "provider": self.provider,
282 "source": self.source,
283 "vendor": None if plan is None else plan.vendor,
284 "model": None if plan is None else plan.model,
285 "transport": None if plan is None else plan.transport,
286 "read_only_backed": None if plan is None else plan.read_only_backed,
287 "eligible": self.eligible,
288 "refusal": self.refusal,
289 }
292def reviewer_id(slot: str, vendor: str) -> str:
293 return artifacts.slug(f"swarm-review-{slot}-{vendor}")
296def plan_review_seat(
297 seat: Mapping[str, Any],
298 *,
299 config: Any,
300 registry: providers_mod.Registry | None,
301 prompt_path: str,
302 cwd: str,
303 timeout: int = delegate.DEFAULT_TIMEOUT_S,
304) -> tuple[delegate.RunPlan | None, str]:
305 """A reviewer seat as a read-only delegate plan, or ``(None, reason)``.
307 The sibling of :func:`keel.swarm_worker.plan_implementer`: resolved and planned exactly
308 as ``keel delegate run --provider <seat> --role review`` would, in the seat's own
309 checkout of the head. What is refused, and why:
311 - a host subagent (``subagent:…``), which only an agent host can spawn;
312 - a seat keel cannot plan (an unknown provider, a bad model token);
313 - a seat whose plan is not **backed** read-only
314 (:attr:`keel.delegate.RunPlan.read_only_backed`): a ``knobs.delegate_profiles``
315 entry with no ``review_args``, which would run with the implementer's write flags.
317 Every other transport is accepted. The three built-in CLIs carry their vendor's
318 documented read-only invocation and read the code around the diff in the checkout. An
319 ``api``/``ollama`` seat has no tools at all — it cannot write, and it cannot read the
320 checkout either, but its brief carries the diff and the issue text, which is what a
321 review needs; it reviews the diff alone.
322 """
323 if seat.get("kind") != "provider":
324 return None, (
325 f"the reviewer seat {seat.get('provider')!r} is a host subagent, which only an "
326 "agent host can spawn; name a provider for it with --review-delegate"
327 )
328 token = seat_token(seat)
329 try:
330 resolution = delegate.resolve_provider(config, registry, token)
331 plan = delegate.plan_run(
332 resolution.provider,
333 REVIEW_ROLE,
334 prompt_path,
335 cwd,
336 timeout,
337 seat.get("effort"),
338 resolution.model,
339 profile=resolution.profile,
340 )
341 except delegate.DelegateError as exc:
342 return None, f"the reviewer seat {token!r} cannot be planned ({exc.code}): {exc.message}"
343 if not plan.read_only_backed:
344 return None, (
345 f"nothing makes the reviewer seat {token!r} read-only (a profile with no "
346 "review_args runs with the implementer's flags); set review_args for it, or "
347 "name another provider with --review-delegate"
348 )
349 return plan, ""
352def review_seats(
353 assignment: Mapping[str, Any] | None,
354 *,
355 config: Any,
356 registry: providers_mod.Registry | None,
357 implementer_vendor: str,
358 brief_path: Callable[[str], str],
359 checkout_path: Callable[[str], str],
360 timeout: int = delegate.DEFAULT_TIMEOUT_S,
361) -> tuple[ReviewSeat, ...]:
362 """The cluster's reviewer seats, each planned or refused.
364 ``brief_path`` and ``checkout_path`` give a slot's brief file and its checkout of the
365 head. A seat from the implementer's own vendor is refused: the owner's decision on #1423
366 is a review from a different vendor than the one that wrote the change, which the review
367 contract states as ``self_review_counts_toward_lgtm: false``.
368 """
369 seats: list[ReviewSeat] = []
370 raw = (assignment or {}).get("reviewers") or ()
371 for index, seat in enumerate(s for s in raw if isinstance(s, Mapping)):
372 slot = str(seat.get("slot") or chr(ord("A") + index))
373 plan, refusal = plan_review_seat(
374 seat,
375 config=config,
376 registry=registry,
377 prompt_path=brief_path(slot),
378 cwd=checkout_path(slot),
379 timeout=timeout,
380 )
381 vendor = plan.vendor if plan is not None else str(seat.get("name") or "seat")
382 if plan is not None and implementer_vendor and plan.vendor == implementer_vendor:
383 refusal = (
384 f"its vendor {plan.vendor!r} is the implementer's; a review from the vendor "
385 "that wrote the change is not an independent opinion"
386 )
387 seats.append(
388 ReviewSeat(
389 slot=slot,
390 reviewer=reviewer_id(slot, vendor),
391 provider=seat_token(seat),
392 source=str(seat.get("source") or ""),
393 plan=plan,
394 refusal=refusal,
395 )
396 )
397 return tuple(seats)
400def cluster_refusal(
401 seats: Sequence[ReviewSeat],
402 *,
403 panel: str,
404 required: int,
405 require_distinct: bool,
406) -> str:
407 """Why keel will not review this cluster at all; ``""`` when it will.
409 Decided before any seat runs, so a cluster that could never post enough verdicts spends
410 nothing. ``required`` is the count ``keel review`` refuses to under-post; at least one
411 seat is always required.
412 """
413 if panel == team.JURY_PANEL:
414 return (
415 "the tier's review is the jury panel, which swarm-review does not convene; run "
416 "the panel and post its ballots with `keel review --from-jury`"
417 )
418 eligible = [s for s in seats if s.eligible]
419 needed = max(required, 1)
420 if len(eligible) < needed:
421 why = "; ".join(f"seat {s.slot} ({s.provider}): {s.refusal}" for s in seats if s.refusal)
422 return (
423 f"the tier requires at least {needed} review verdict(s), and only {len(eligible)} "
424 f"of the cluster's {len(seats)} reviewer seat(s) can review it"
425 + (f" — {why}" if why else "")
426 + "; name providers per slot with --review-delegate"
427 )
428 if require_distinct:
429 check = evidence.distinct_vendor_check(
430 [s.vendor for s in eligible], required_count=len(eligible)
431 )
432 if not check["ok"]:
433 return (
434 f"require_distinct_vendors is on and the seats would fail it ({check['reason']}); "
435 "every posted verdict must come from a different vendor — name another "
436 "provider for the duplicated slot with --review-delegate"
437 )
438 return ""
441def _fence(text: str) -> str:
442 """A backtick fence longer than any run of backticks in ``text``."""
443 longest = max((len(run) for run in re.findall(r"`+", text)), default=0)
444 return "`" * max(3, longest + 1)
447def render_review_brief(
448 *,
449 swarm_id: str,
450 cluster_id: str,
451 pull_request: int,
452 facts: PullRequestFacts,
453 seat: ReviewSeat,
454 focus: Sequence[str],
455 issues: Sequence[tuple[int, str, str]],
456) -> str:
457 """One seat's brief: ``/keel:ship`` s7's reviewer briefing, for one cluster pull request.
459 The head is pinned, the diff is the one keel read at it, and the answer is one JSON
460 object :func:`read_seat_verdict` parses. ``issues`` are ``(number, title, body)``.
461 """
462 reviewers = _reviewers(facts.contract)
463 lines = [
464 (
465 f"You are reviewer {seat.slot} of pull request #{pull_request} (cluster "
466 f"{cluster_id} of keel swarm {swarm_id}), at head {facts.head_sha}."
467 ),
468 (
469 "Your working directory is a checkout of exactly that head. Read the code around "
470 "the diff there; review only that head."
471 ),
472 "",
473 "## Rules",
474 "",
475 (
476 "- You are read-only. Do not edit, create or delete files, commit, push, or run "
477 "anything that changes the repository or its remote. keel posts your verdict "
478 "itself."
479 ),
480 (
481 "- You have no GitHub credentials and git here cannot reach any remote, by "
482 "design; do not work around it."
483 ),
484 ("- Review independently: do not look for, read or wait on any other reviewer's output."),
485 "",
486 "## Stance",
487 "",
488 *(f"- {line}" for line in REVIEW_STANCE),
489 "",
490 "## Your focus",
491 "",
492 *(f"- {dim}" for dim in focus or ("every dimension of the change",)),
493 ]
494 additions = _strings(reviewers.get("project_additions"))
495 if additions:
496 lines += [
497 "",
498 "## Recurring shapes in this project (look for these; not a checklist)",
499 "",
500 *(f"- {entry}" for entry in additions),
501 ]
502 sections = _strings(reviewers.get("required_sections"))
503 if sections:
504 lines += [
505 "",
506 "## Sections your scope must cover",
507 "",
508 *(f"- {entry}" for entry in sections),
509 ]
510 for number, title, body in issues:
511 lines += ["", f"## Issue #{number}: {title or '(title unavailable)'}", ""]
512 lines.append(body or "(The issue body could not be read; work from the title.)")
513 diff = facts.diff
514 note = ""
515 if len(diff) > MAX_DIFF_CHARS:
516 diff = diff[:MAX_DIFF_CHARS]
517 note = f"(The diff is cut at {MAX_DIFF_CHARS} characters; read the rest in your checkout.)"
518 fence = _fence(diff)
519 lines += [
520 "",
521 f"## The diff at {facts.head_sha}",
522 "",
523 f"{fence}diff",
524 diff.rstrip("\n"),
525 fence,
526 ]
527 if note:
528 lines.append(note)
529 lines += [
530 "",
531 "## Your answer",
532 "",
533 "End your answer with exactly one JSON object, and nothing after it:",
534 "",
535 "```json",
536 json.dumps(
537 {
538 "verdict": "APPROVE or REQUEST_CHANGES",
539 "scope": "what you checked, naming the files, functions or paths",
540 "findings": [
541 {
542 "severity": "critical | major | minor | nit",
543 "message": "the defect, demonstrated",
544 "path": "src/file.py",
545 "line": 42,
546 }
547 ],
548 "testing": "what the tests do and do not pin",
549 },
550 indent=2,
551 ),
552 "```",
553 "",
554 (
555 "Any critical or major finding means REQUEST_CHANGES. `findings` is an empty "
556 "list for a clean review, whose `scope` says what you checked. An answer keel "
557 "cannot parse is recorded as a failed review, never as an approval."
558 ),
559 ]
560 return "\n".join(lines) + "\n"
563@dataclass(frozen=True)
564class SeatVerdict:
565 """What one seat answered, as keel read it."""
567 slot: str
568 reviewer: str
569 #: :data:`APPROVE`, :data:`REQUEST_CHANGES` or :data:`FAILED`.
570 outcome: str
571 reason: str = ""
572 findings: tuple[Mapping[str, Any], ...] = ()
573 #: The ``keel review --reviews`` entry for this verdict; ``None`` for a failed one.
574 item: Mapping[str, Any] | None = None
575 #: The seat changed the repository's git setup: nothing of the cluster is posted.
576 tampered: bool = False
578 def to_dict(self) -> dict[str, Any]:
579 return {
580 "slot": self.slot,
581 "reviewer": self.reviewer,
582 "outcome": self.outcome,
583 "reason": self.reason,
584 "findings": [dict(f) for f in self.findings],
585 "tampered": self.tampered,
586 }
589def failed_verdict(seat: ReviewSeat, reason: str, *, tampered: bool = False) -> SeatVerdict:
590 return SeatVerdict(seat.slot, seat.reviewer, FAILED, reason, tampered=tampered)
593def extract_verdict_object(text: str) -> dict[str, Any] | None:
594 """The last JSON object in ``text`` that carries a ``verdict``, or ``None``.
596 A seat answers in prose and ends with the object, often inside a fenced block; every
597 ``{`` is tried as the start of one, and the last object with a ``verdict`` key wins.
598 """
599 decoder = json.JSONDecoder()
600 found: dict[str, Any] | None = None
601 for match in re.finditer(r"\{", text):
602 try:
603 value, _end = decoder.raw_decode(text, match.start())
604 except ValueError:
605 continue
606 if isinstance(value, dict) and "verdict" in value:
607 found = value
608 return found
611def _finding_issue(finding: Any, index: int) -> str:
612 if not isinstance(finding, Mapping):
613 return f"finding #{index + 1} is not an object"
614 severity = finding.get("severity")
615 if not isinstance(severity, str) or severity.strip().lower() not in fnd.SEVERITIES:
616 valid = ", ".join(fnd.SEVERITIES)
617 return f"finding #{index + 1} has severity {severity!r}, not one of {valid}"
618 message = finding.get("message")
619 if not isinstance(message, str) or not message.strip():
620 return f"finding #{index + 1} has no message"
621 return ""
624#: How much of a malformed finding a carried finding quotes.
625MAX_QUOTED_CHARS = 500
628def _quoted(value: Any) -> str:
629 text = value if isinstance(value, str) else json.dumps(value, sort_keys=True, default=str)
630 return text if len(text) <= MAX_QUOTED_CHARS else text[:MAX_QUOTED_CHARS] + "…"
633def _normalised(finding: Mapping[str, Any]) -> dict[str, Any]:
634 return {**finding, "severity": str(finding["severity"]).strip().lower()}
637def _rejection_findings(raw: Any) -> list[dict[str, Any]]:
638 """A rejecting seat's findings, every one kept: a well-formed finding as it is, anything
639 else carried as one ``major`` finding quoting what the seat wrote."""
640 if raw is None:
641 return []
642 entries = raw if isinstance(raw, list) else [raw]
643 findings: list[dict[str, Any]] = []
644 for index, finding in enumerate(entries):
645 if _finding_issue(finding, index):
646 findings.append(
647 {
648 "severity": "major",
649 "message": f"the seat's finding, as it wrote it: {_quoted(finding)}",
650 }
651 )
652 else:
653 findings.append(_normalised(finding))
654 return findings
657def _entry(seat: ReviewSeat, verdict: Any, scope: Any, findings: Any, testing: Any) -> dict:
658 plan = seat.plan
659 return {
660 "reviewer": seat.reviewer,
661 "verdict": verdict,
662 "scope": scope,
663 "findings": findings,
664 "testing": testing,
665 "vendor": None if plan is None else plan.vendor,
666 "model": None if plan is None else plan.model,
667 }
670def _posted(item: review.ReviewItem, verdict: str) -> dict[str, Any]:
671 return {
672 "reviewer": item.reviewer,
673 "verdict": verdict,
674 "scope": item.scope,
675 "findings": [dict(f) for f in item.findings],
676 "testing": item.testing,
677 "vendor": item.vendor,
678 "model": item.model,
679 }
682def _rejection(
683 seat: ReviewSeat, obj: Mapping[str, Any], *, head_sha: str, reason: str
684) -> SeatVerdict:
685 """A seat that expressed a rejection, posted as ``REQUEST_CHANGES`` with what is usable.
687 Never discarded: a rejection whose scope is empty, whose findings are malformed or whose
688 prose is thin still holds ``keel merge`` once posted, and dropping it would let the other
689 seats' approvals land the change it rejected. The scope falls back to a sentence keel
690 writes; a malformed finding is carried as one ``major`` finding quoting it.
691 """
692 vendor = seat.vendor or "unknown vendor"
693 scope = obj.get("scope")
694 if not isinstance(scope, str) or not scope.strip():
695 scope = (
696 f"swarm-review seat {seat.slot} ({vendor}) requested changes at {head_sha}; its "
697 "answer did not name what it checked"
698 )
699 testing = obj.get("testing")
700 entry = _entry(
701 seat,
702 REQUEST_CHANGES,
703 scope,
704 _rejection_findings(obj.get("findings")),
705 testing if isinstance(testing, str) else None,
706 )
707 # Built so it always parses: the loader is the one `keel review --reviews` runs.
708 (item,) = review.parse_reviews([entry])
709 return SeatVerdict(
710 seat.slot,
711 seat.reviewer,
712 REQUEST_CHANGES,
713 reason,
714 item.findings,
715 _posted(item, REQUEST_CHANGES),
716 )
719def verdict_from_object(
720 seat: ReviewSeat, obj: Mapping[str, Any], *, head_sha: str, pr_title: str = ""
721) -> SeatVerdict:
722 """One seat's JSON verdict, read the way the evidence gate reads a posted one.
724 The verdict word is read first, with the gate's own :func:`keel.evidence.review_verdict_token`:
726 - **It does not approve** (``REQUEST_CHANGES``, ``COMMENT``, ``ABSTAIN``, ``REJECT`` …,
727 anything outside :data:`keel.evidence.APPROVING_VERDICTS`): the seat rejected, and its
728 verdict is posted as ``REQUEST_CHANGES`` whatever the rest of its answer looks like
729 (:func:`_rejection`). A seat that expressed a rejection is never discarded.
730 - **It approves:** the answer must pass everything a counted approval needs — the
731 :func:`keel.review.parse_reviews` loader, a non-empty scope, findings in keel's severity
732 vocabulary, and a body :func:`keel.evidence.verdict_substance` would count — or the seat
733 is :data:`FAILED`. An approval carrying a critical or major finding is a rejection.
734 - **No word at all** (a missing, empty or non-string verdict): nothing was expressed, and
735 the seat is :data:`FAILED`.
737 keel, not the seat, names the reviewer, vendor and model — from the seat's attribution.
738 """
739 raw = obj.get("verdict")
740 token = evidence.review_verdict_token(f"Verdict: {raw}") if isinstance(raw, str) else None
741 if token is None:
742 return failed_verdict(seat, f"its verdict {raw!r} names no verdict")
743 if token not in evidence.APPROVING_VERDICTS:
744 reason = (
745 ""
746 if token in evidence.REQUEST_CHANGES_VERDICTS
747 else f"its verdict {token} does not approve, so it is posted as REQUEST_CHANGES"
748 )
749 return _rejection(seat, obj, head_sha=head_sha, reason=reason)
750 try:
751 (item,) = review.parse_reviews(
752 [_entry(seat, raw, obj.get("scope"), obj.get("findings"), obj.get("testing"))]
753 )
754 except review.ReviewError as exc:
755 return failed_verdict(seat, f"its approval does not parse: {exc}")
756 for index, finding in enumerate(item.findings):
757 if why := _finding_issue(finding, index):
758 return failed_verdict(seat, f"its approval does not parse: {why}")
759 findings = [_normalised(finding) for finding in item.findings]
760 if any(fnd.decision_for(f["severity"]) == "block" for f in findings):
761 return _rejection(
762 seat,
763 {**obj, "findings": findings},
764 head_sha=head_sha,
765 reason=(
766 "it approved with a critical or major finding, which is posted as REQUEST_CHANGES"
767 ),
768 )
769 if not item.scope or not item.scope.strip():
770 return failed_verdict(seat, "its approval says nothing about what it checked (no scope)")
771 body = artifacts.render_review_verdict(
772 reviewer=item.reviewer,
773 head_sha=head_sha,
774 verdict=APPROVE,
775 scope=item.scope,
776 findings=findings,
777 testing=item.testing,
778 vendor=item.vendor,
779 model=item.model,
780 )
781 ok, why = evidence.verdict_substance(body, pr_title=pr_title)
782 if not ok:
783 return failed_verdict(
784 seat, f"its approval names nothing concrete, so keel merge would not count it: {why}"
785 )
786 item = replace(item, findings=tuple(findings))
787 outcome = APPROVE if evidence.verdict_approves(body) else REQUEST_CHANGES
788 return SeatVerdict(seat.slot, seat.reviewer, outcome, "", item.findings, _posted(item, outcome))
791def read_seat_verdict(
792 seat: ReviewSeat, result: Mapping[str, Any], *, head_sha: str, pr_title: str = ""
793) -> SeatVerdict:
794 """A seat's ``keel delegate run`` result as its verdict: a failed run or an answer with no
795 JSON verdict in it is :data:`FAILED`."""
796 if not result.get("ok"):
797 return failed_verdict(
798 seat,
799 f"the seat did not answer ({result.get('error_code')}): {result.get('error')}",
800 )
801 obj = extract_verdict_object(str(result.get("text") or ""))
802 if obj is None:
803 return failed_verdict(seat, "its answer carries no JSON object with a verdict")
804 return verdict_from_object(seat, obj, head_sha=head_sha, pr_title=pr_title)
807def posting_decision(verdicts: Sequence[SeatVerdict], *, required: int) -> str:
808 """Why nothing of a cluster is posted; ``""`` when :func:`posted_items` is.
810 The rule (#1423, after #1426 made the evidence gate read each verdict):
812 - A seat that changed the repository's git setup holds the whole cluster.
813 - A rejection is always posted. When some seats approve and one requests changes, all of
814 them are posted: the gate reads each reviewer's latest verdict at the head, so the
815 rejection holds ``keel merge`` with ``review-verdict-not-approved`` however many others
816 approved.
817 - **Fail closed on a seat that did not answer readably** (:data:`FAILED`). It may have been
818 about to reject, and landing on the remaining approvals would review the pull request
819 with fewer eyes than the plan staffed. So with no rejection, the cluster posts nothing
820 and is held; with a rejection, only the rejection(s) are posted — they hold anyway.
821 - With every seat approving, fewer approvals than the tier requires post nothing: ``keel
822 review`` refuses an under-count bundle of approvals, and it would not land anyway.
823 """
824 tampered = [v.slot for v in verdicts if v.tampered]
825 if tampered:
826 return (
827 f"seat(s) {', '.join(tampered)} changed the repository's git setup while they ran; "
828 "nothing is posted — inspect the repository's git config and hooks"
829 )
830 if any(v.outcome == REQUEST_CHANGES for v in verdicts):
831 return ""
832 failed = [v.slot for v in verdicts if v.outcome == FAILED]
833 if failed:
834 return (
835 f"seat(s) {', '.join(failed)} did not return a readable verdict, so nothing is "
836 "posted: a seat that did not answer may have been about to reject — rerun "
837 "swarm-review"
838 )
839 needed = max(required, 1)
840 if len(verdicts) < needed:
841 return (
842 f"{len(verdicts)} seat(s) approved and the tier requires at least {needed} "
843 "verdict(s), so nothing is posted"
844 )
845 return ""
848def posted_status(verdicts: Sequence[SeatVerdict]) -> str:
849 """:data:`POSTED_CHANGES_REQUESTED` when a posted verdict requests changes, else
850 :data:`POSTED`."""
851 if any(v.outcome == REQUEST_CHANGES for v in verdicts):
852 return POSTED_CHANGES_REQUESTED
853 return POSTED
856def head_moved(reviewed: str, current: str) -> str:
857 """Why nothing is posted when the head the seats reviewed is no longer the head."""
858 if current == reviewed:
859 return ""
860 return (
861 f"the pull request's head moved from {reviewed} to {current or 'an unreadable head'} "
862 "while the seats ran; nothing is posted — run swarm-review again on the new head"
863 )
866def review_run_id(swarm_id: str, cluster_id: str) -> str:
867 """The ``keel review --run-id`` of a cluster: its provenance run id, so each seat's
868 verdict is the comment ``<run-id>:rv-<reviewer>`` and a second run edits it in place."""
869 return swarm_worker.provenance_run_id(swarm_id, cluster_id)
872@dataclass(frozen=True)
873class ClusterReview:
874 """What ``swarm-review`` did, or would do, for one cluster."""
876 cluster_id: str
877 #: One of :data:`POSTED`, :data:`POSTED_CHANGES_REQUESTED`, :data:`PLANNED`,
878 #: :data:`HELD`, :data:`REFUSED`,
879 #: :data:`SKIPPED`, :data:`MERGED` or :data:`ERROR`.
880 status: str
881 reason: str = ""
882 pull_request: int | None = None
883 head_sha: str = ""
884 tier: int | None = None
885 required: int = 0
886 seats: tuple[ReviewSeat, ...] = ()
887 verdicts: tuple[SeatVerdict, ...] = ()
888 warnings: tuple[str, ...] = ()
890 @property
891 def posted(self) -> bool:
892 return self.status in (POSTED, POSTED_CHANGES_REQUESTED)
894 def to_dict(self) -> dict[str, Any]:
895 return {
896 "cluster_id": self.cluster_id,
897 "status": self.status,
898 "posted": self.posted,
899 "reason": self.reason,
900 "pull_request": self.pull_request,
901 "head_sha": self.head_sha,
902 "tier": self.tier,
903 "required": self.required,
904 "seats": [s.to_dict() for s in self.seats],
905 "verdicts": [v.to_dict() for v in self.verdicts],
906 "warnings": list(self.warnings),
907 }
910@dataclass(frozen=True)
911class SwarmReviewResult:
912 swarm_id: str
913 wave_index: int
914 dry_run: bool
915 clusters: tuple[ClusterReview, ...] = ()
916 warnings: tuple[str, ...] = field(default_factory=tuple)
918 @property
919 def status(self) -> str:
920 """``success`` when every cluster ended clean (:data:`CLEAN_STATUSES`) and there was
921 at least one; otherwise ``failed``."""
922 if self.clusters and all(c.status in CLEAN_STATUSES for c in self.clusters):
923 return "success"
924 return "failed"
926 def to_dict(self) -> dict[str, Any]:
927 return {
928 "swarm_id": self.swarm_id,
929 "wave_index": self.wave_index,
930 "dry_run": self.dry_run,
931 "status": self.status,
932 "clusters": [c.to_dict() for c in self.clusters],
933 "warnings": list(self.warnings),
934 }
937def _seat_line(seat: ReviewSeat, verdict: SeatVerdict | None) -> list[str]:
938 plan = seat.plan
939 where = f"{seat.provider}" + (f" over {plan.transport}" if plan is not None else "")
940 if not seat.eligible:
941 return [f" seat {seat.slot} {where}: refused — {seat.refusal}"]
942 if verdict is None:
943 return [f" seat {seat.slot} {where}: would review as {seat.reviewer}"]
944 line = f" seat {seat.slot} {where}: {verdict.outcome}"
945 if verdict.reason:
946 line += f" — {verdict.reason}"
947 out = [line]
948 out += [f" - {f['severity']}: {f['message']}" for f in verdict.findings]
949 return out
952def render_swarm_review_result(result: SwarmReviewResult) -> str:
953 mode = "dry-run" if result.dry_run else "live"
954 head = f"keel swarm-review — {mode} swarm {result.swarm_id}, wave {result.wave_index}"
955 lines = [f"{head}: {result.status}"]
956 if not result.clusters:
957 lines.append(" no cluster in this wave")
958 for cluster in result.clusters:
959 where = f"PR #{cluster.pull_request}" if cluster.pull_request is not None else "no PR"
960 if cluster.head_sha:
961 where += f" @ {cluster.head_sha[:12]}"
962 lines.append(f" {cluster.cluster_id}: {where} — {cluster.status}")
963 if cluster.required:
964 lines.append(f" tier {cluster.tier}: requires {cluster.required} verdict(s)")
965 verdicts = {v.slot: v for v in cluster.verdicts}
966 for seat in cluster.seats:
967 lines += _seat_line(seat, verdicts.get(seat.slot))
968 if cluster.reason:
969 lines.append(f" {cluster.reason}")
970 lines += [f" warning: {w}" for w in cluster.warnings]
971 lines += [f" warning: {w}" for w in result.warnings]
972 return "\n".join(lines)
975def with_status(review_: ClusterReview, status: str, reason: str) -> ClusterReview:
976 return replace(review_, status=status, reason=reason)
979def posted_items(verdicts: Sequence[SeatVerdict]) -> list[dict[str, Any]]:
980 """The ``keel review --reviews`` bundle, in seat order: every readable verdict — or, when a
981 seat failed, the rejections alone (:func:`posting_decision`). A failed seat has none."""
982 failed = any(v.outcome == FAILED for v in verdicts)
983 return [
984 dict(v.item)
985 for v in verdicts
986 if v.item is not None and not (failed and v.outcome == APPROVE)
987 ]
990#: How much of a reason the run state keeps per cluster and per seat (#1440).
991RECORD_REASON_CHARS = 300
994def _bounded(text: str) -> str:
995 return text if len(text) <= RECORD_REASON_CHARS else text[:RECORD_REASON_CHARS] + "…"
998def review_record(cluster: ClusterReview, *, run_id: str, reviewed_at: str) -> dict[str, Any]:
999 """What a live review leaves in the cluster's worker record (#1440).
1001 ``keel swarm-status`` shows it. Its status, the head the seats reviewed, the tier's
1002 count, each seat — slot, reviewer, vendor and outcome (``APPROVE``, ``REQUEST_CHANGES``,
1003 ``failed``; ``refused`` for a seat keel would not run, ``not-run`` for one the cluster
1004 never reached) — when, and the ``keel review`` run id the verdicts were posted under.
1005 Reasons are bounded. A record, not evidence: only the comments on the pull request are.
1006 """
1007 verdicts = {v.slot: v for v in cluster.verdicts}
1008 seats = []
1009 for seat in cluster.seats:
1010 verdict = verdicts.get(seat.slot)
1011 if verdict is not None:
1012 outcome, reason = verdict.outcome, verdict.reason
1013 else:
1014 outcome = "not-run" if seat.eligible else "refused"
1015 reason = seat.refusal
1016 seats.append(
1017 {
1018 "slot": seat.slot,
1019 "reviewer": seat.reviewer,
1020 "vendor": seat.vendor or seat.provider,
1021 "outcome": outcome,
1022 "reason": _bounded(reason),
1023 }
1024 )
1025 return {
1026 "status": cluster.status,
1027 "reason": _bounded(cluster.reason),
1028 "pull_request": cluster.pull_request,
1029 "head_sha": cluster.head_sha,
1030 "tier": cluster.tier,
1031 "required": cluster.required,
1032 "run_id": run_id,
1033 "reviewed_at": reviewed_at,
1034 "seats": seats,
1035 }