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

1"""What ``keel swarm-review`` decides about a cluster's pull request (#1423). 

2 

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. 

7 

8This module holds every *decision* in that step, and nothing else: 

9 

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. 

35 

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""" 

39 

40from __future__ import annotations 

41 

42import json 

43import re 

44from collections.abc import Callable, Mapping, Sequence 

45from dataclasses import dataclass, field, replace 

46from typing import Any, NamedTuple 

47 

48from . import artifacts, consent, delegate, evidence, review, swarm_worker, team 

49from . import findings as fnd 

50from . import providers as providers_mod 

51 

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") 

55 

56#: The role every seat runs in: :data:`keel.delegate.READ_ONLY_ROLES`. 

57REVIEW_ROLE = "review" 

58 

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" 

67 

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) 

83 

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 

87 

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) 

110 

111 

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) 

115 

116 

117def consent_refusal(contract: Mapping[str, Any]) -> str: 

118 """Why a live ``swarm-review`` may not start under ``contract``; ``""`` when it may. 

119 

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 "" 

137 

138 

139class PullRequestFacts(NamedTuple): 

140 """What keel read from a cluster's pull request before any seat runs.""" 

141 

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 = "" 

151 

152 

153def _reviewers(contract: Mapping[str, Any]) -> Mapping[str, Any]: 

154 block = contract.get("reviewers") 

155 return block if isinstance(block, Mapping) else {} 

156 

157 

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 

162 

163 

164def distinct_vendors_required(contract: Mapping[str, Any]) -> bool: 

165 return _reviewers(contract).get("require_distinct_vendors") is True 

166 

167 

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()) 

172 

173 

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``. 

176 

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"))) 

189 

190 

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. 

193 

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 

203 

204 

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`). 

207 

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)) 

221 

222 

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 "") 

227 

228 

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. 

237 

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() 

251 

252 

253@dataclass(frozen=True) 

254class ReviewSeat: 

255 """One reviewer seat of a cluster, as keel would dispatch it, or why it would not.""" 

256 

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 = "" 

267 

268 @property 

269 def eligible(self) -> bool: 

270 return self.plan is not None and not self.refusal 

271 

272 @property 

273 def vendor(self) -> str | None: 

274 return None if self.plan is None else self.plan.vendor 

275 

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 } 

290 

291 

292def reviewer_id(slot: str, vendor: str) -> str: 

293 return artifacts.slug(f"swarm-review-{slot}-{vendor}") 

294 

295 

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)``. 

306 

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: 

310 

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. 

316 

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, "" 

350 

351 

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. 

363 

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) 

398 

399 

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. 

408 

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 "" 

439 

440 

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) 

445 

446 

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. 

458 

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" 

561 

562 

563@dataclass(frozen=True) 

564class SeatVerdict: 

565 """What one seat answered, as keel read it.""" 

566 

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 

577 

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 } 

587 

588 

589def failed_verdict(seat: ReviewSeat, reason: str, *, tampered: bool = False) -> SeatVerdict: 

590 return SeatVerdict(seat.slot, seat.reviewer, FAILED, reason, tampered=tampered) 

591 

592 

593def extract_verdict_object(text: str) -> dict[str, Any] | None: 

594 """The last JSON object in ``text`` that carries a ``verdict``, or ``None``. 

595 

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 

609 

610 

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 "" 

622 

623 

624#: How much of a malformed finding a carried finding quotes. 

625MAX_QUOTED_CHARS = 500 

626 

627 

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] + "…" 

631 

632 

633def _normalised(finding: Mapping[str, Any]) -> dict[str, Any]: 

634 return {**finding, "severity": str(finding["severity"]).strip().lower()} 

635 

636 

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 

655 

656 

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 } 

668 

669 

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 } 

680 

681 

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. 

686 

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 ) 

717 

718 

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. 

723 

724 The verdict word is read first, with the gate's own :func:`keel.evidence.review_verdict_token`: 

725 

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`. 

736 

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)) 

789 

790 

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) 

805 

806 

807def posting_decision(verdicts: Sequence[SeatVerdict], *, required: int) -> str: 

808 """Why nothing of a cluster is posted; ``""`` when :func:`posted_items` is. 

809 

810 The rule (#1423, after #1426 made the evidence gate read each verdict): 

811 

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 "" 

846 

847 

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 

854 

855 

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 ) 

864 

865 

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) 

870 

871 

872@dataclass(frozen=True) 

873class ClusterReview: 

874 """What ``swarm-review`` did, or would do, for one cluster.""" 

875 

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, ...] = () 

889 

890 @property 

891 def posted(self) -> bool: 

892 return self.status in (POSTED, POSTED_CHANGES_REQUESTED) 

893 

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 } 

908 

909 

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) 

917 

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" 

925 

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 } 

935 

936 

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 

950 

951 

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) 

973 

974 

975def with_status(review_: ClusterReview, status: str, reason: str) -> ClusterReview: 

976 return replace(review_, status=status, reason=reason) 

977 

978 

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 ] 

988 

989 

990#: How much of a reason the run state keeps per cluster and per seat (#1440). 

991RECORD_REASON_CHARS = 300 

992 

993 

994def _bounded(text: str) -> str: 

995 return text if len(text) <= RECORD_REASON_CHARS else text[:RECORD_REASON_CHARS] + "…" 

996 

997 

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). 

1000 

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 }