Coverage for src/keel/review.py: 100%
126 statements
« prev ^ index » next coverage.py v7.16.2, created at 2026-10-02 20:26 +0000
1"""Pure orchestration for ``keel review`` — the evidence-bundle orchestrator.
3The host agent runs the actual reviewers and produces the review *content*. This
4module takes that supplied content and deterministically decides what to render
5and where to post it: one head-pinned reviewer verdict per supplied review, an
6optional closure comment posted to both the PR and the linked issue, and the
7run-id sub-keys that bind each post to a stable, idempotent comment.
9Everything here is pure — no network, no subprocess, no clock, no randomness.
10Rendering uses the already-pure artifact/closure renderers. The CLI handler owns
11the head-SHA fetch, the actual posting, and the optional re-verify.
12"""
14from __future__ import annotations
16from dataclasses import dataclass, field
17from typing import Any
19from . import artifacts, closure, evidence
21SCHEMA_VERSION = "keel.review.v1"
24class ReviewError(ValueError):
25 """Raised when the supplied review bundle is malformed or under-count."""
28@dataclass(frozen=True)
29class ReviewItem:
30 """One parsed reviewer verdict supplied by the host agent."""
32 reviewer: str
33 verdict: str
34 scope: str | None
35 findings: tuple[dict[str, Any], ...]
36 testing: str | None
37 vendor: str | None = None
38 model: str | None = None
41@dataclass(frozen=True)
42class PostTarget:
43 """A single planned post: a rendered body bound to a target and run-id sub-key."""
45 artifact: str
46 target_kind: str
47 target_number: int
48 run_id: str
49 marker: str
50 body: str
52 def as_dict(self) -> dict[str, Any]:
53 return {
54 "artifact": self.artifact,
55 "target": {"kind": self.target_kind, "number": self.target_number},
56 "run_id": self.run_id,
57 "marker": self.marker,
58 "body": self.body,
59 }
62@dataclass(frozen=True)
63class ReviewPlan:
64 """The deterministic plan: rendered verdicts, optional closure, post targets."""
66 pull_request: int
67 issue: int | None
68 head_sha: str | None
69 run_id: str
70 tier: int | None
71 required_count: int
72 supplied_count: int
73 posts: tuple[PostTarget, ...] = field(default_factory=tuple)
75 def as_dict(self) -> dict[str, Any]:
76 return {
77 "schema_version": SCHEMA_VERSION,
78 "pull_request": self.pull_request,
79 "issue": self.issue,
80 "head_sha": self.head_sha,
81 "run_id": self.run_id,
82 "tier": self.tier,
83 "required_count": self.required_count,
84 "supplied_count": self.supplied_count,
85 "posts": [post.as_dict() for post in self.posts],
86 }
89def parse_reviews(raw: object) -> tuple[ReviewItem, ...]:
90 """Parse and validate a ``--reviews`` JSON payload into ``ReviewItem`` records."""
91 if not isinstance(raw, list):
92 raise ReviewError("reviews file must contain a JSON array of review objects")
93 items: list[ReviewItem] = []
94 for index, entry in enumerate(raw):
95 items.append(_parse_review(entry, index))
96 return tuple(items)
99def _parse_review(entry: object, index: int) -> ReviewItem:
100 if not isinstance(entry, dict):
101 raise ReviewError(f"review #{index + 1} must be a JSON object")
102 reviewer = entry.get("reviewer")
103 if not isinstance(reviewer, str) or not reviewer.strip():
104 raise ReviewError(f"review #{index + 1} requires a non-empty 'reviewer' string")
105 verdict = entry.get("verdict")
106 if not isinstance(verdict, str) or not verdict.strip():
107 raise ReviewError(f"review #{index + 1} requires a non-empty 'verdict' string")
108 scope = entry.get("scope")
109 if scope is not None and not isinstance(scope, str):
110 raise ReviewError(f"review #{index + 1} 'scope' must be a string when present")
111 testing = entry.get("testing")
112 if testing is not None and not isinstance(testing, str):
113 raise ReviewError(f"review #{index + 1} 'testing' must be a string when present")
114 vendor = _parse_provenance_field(entry.get("vendor"), index, "vendor")
115 model = _parse_provenance_field(entry.get("model"), index, "model")
116 findings = _parse_findings(entry.get("findings"), index)
117 return ReviewItem(
118 reviewer=reviewer.strip(),
119 verdict=verdict.strip(),
120 scope=scope,
121 findings=findings,
122 testing=testing,
123 vendor=vendor,
124 model=model,
125 )
128def _parse_provenance_field(value: object, index: int, name: str) -> str | None:
129 """Parse an optional ``vendor``/``model`` provenance string from a review entry."""
130 if value is None:
131 return None
132 if not isinstance(value, str):
133 raise ReviewError(f"review #{index + 1} '{name}' must be a string when present")
134 cleaned = value.strip()
135 return cleaned or None
138def _parse_findings(raw: object, index: int) -> tuple[dict[str, Any], ...]:
139 if raw is None:
140 return ()
141 if not isinstance(raw, list):
142 raise ReviewError(f"review #{index + 1} 'findings' must be a list when present")
143 findings: list[dict[str, Any]] = []
144 for finding_index, finding in enumerate(raw):
145 if not isinstance(finding, dict):
146 raise ReviewError(
147 f"review #{index + 1} finding #{finding_index + 1} must be a JSON object"
148 )
149 findings.append(dict(finding))
150 return tuple(findings)
153def parse_cycle_reviewers(raw: object) -> tuple[dict[str, Any], ...]:
154 """Parse a review-cycle findings payload into reviewer records for rendering.
156 The payload is the host-supplied structured block each reviewer returns
157 (codename · focus · verdict · findings · clean areas). Validation is shallow
158 on purpose: the renderer applies per-field fallbacks, so this only enforces
159 the outer shape and surfaces a clear error when it is malformed.
160 """
161 if not isinstance(raw, list):
162 raise ReviewError("review-cycle findings must be a JSON array of reviewer objects")
163 reviewers: list[dict[str, Any]] = []
164 for index, entry in enumerate(raw):
165 if not isinstance(entry, dict):
166 raise ReviewError(f"reviewer #{index + 1} must be a JSON object")
167 reviewers.append(dict(entry))
168 return tuple(reviewers)
171def review_run_id(run_id: str, reviewer: str) -> str:
172 """Stable per-reviewer run-id sub-key, e.g. ``<run-id>:rv-<reviewer-slug>``."""
173 return f"{run_id}:rv-{artifacts.slug(reviewer)}"
176def closure_run_id(run_id: str) -> str:
177 """Stable run-id sub-key for the closure-comment artifact."""
178 return f"{run_id}:closure"
181def jury_run_id(run_id: str) -> str:
182 """Stable run-id sub-key for the jury-verdict artifact."""
183 return f"{run_id}:jury"
186def build_review_plan(
187 reviews: tuple[ReviewItem, ...],
188 *,
189 required_count: int,
190 head_sha: str | None,
191 pull_request: int,
192 issue: int | None,
193 run_id: str,
194 tier: int | None,
195 closure_record: dict[str, Any] | None = None,
196 jury_record: dict[str, Any] | None = None,
197) -> ReviewPlan:
198 """Validate the bundle against the required count and build the post plan.
200 Fewer supplied reviews than required fails; exact or more is allowed — unless the
201 bundle carries a verdict that does not approve (:func:`keel.evidence.verdict_approves`
202 on the verdict as it renders), or a ``jury_record`` whose consensus does not
203 (:func:`keel.evidence.jury_verdict_approves` on the jury verdict as it renders, #1429).
204 Either holds ``keel merge`` on its own (#1426), so refusing it would hide a rejection
205 without protecting anything; an under-count bundle of approvals is still refused
206 (#1423). Each review renders as a head-pinned
207 verdict posted to the PR. A closure record,
208 when supplied, renders once and posts to both the PR and the linked issue.
210 ``jury_record`` is the panel's consensus record (:func:`keel.jury.jury_verdict`),
211 supplied when the reviews *are* a jury panel's ballots (#1015). It renders the
212 one ``keel.jury-verdict.v1`` comment alongside the per-ballot verdicts, so the
213 panel posts its evidence in a single call and the two halves are pinned to the
214 same head SHA by construction — a jury verdict posted separately from the
215 ballots it summarises is the drift this whole path removes.
216 """
217 supplied = len(reviews)
218 jury_post = (
219 _jury_post(jury_record, head_sha=head_sha, pull_request=pull_request, run_id=run_id)
220 if jury_record is not None
221 else None
222 )
223 rejects = any(
224 not evidence.verdict_approves(f"Verdict: {item.verdict}") for item in reviews
225 ) or (jury_post is not None and not evidence.jury_verdict_approves(jury_post.body))
226 if supplied < required_count and not rejects:
227 raise ReviewError(
228 f"supplied {supplied} review(s) but tier requires at least "
229 f"{required_count}; refusing to under-post evidence"
230 )
231 posts: list[PostTarget] = []
232 for item in reviews:
233 body = artifacts.render_review_verdict(
234 reviewer=item.reviewer,
235 head_sha=head_sha,
236 verdict=item.verdict,
237 scope=item.scope,
238 findings=list(item.findings),
239 testing=item.testing,
240 vendor=item.vendor,
241 model=item.model,
242 )
243 posts.append(
244 PostTarget(
245 artifact="review-verdict",
246 target_kind="pr",
247 target_number=pull_request,
248 run_id=review_run_id(run_id, item.reviewer),
249 marker=evidence.REVIEW_VERDICT_MARKER,
250 body=body,
251 )
252 )
253 if jury_post is not None:
254 posts.append(jury_post)
255 if closure_record is not None:
256 posts.extend(
257 _closure_posts(
258 closure_record,
259 pull_request=pull_request,
260 issue=issue,
261 run_id=run_id,
262 )
263 )
264 return ReviewPlan(
265 pull_request=pull_request,
266 issue=issue,
267 head_sha=head_sha,
268 run_id=run_id,
269 tier=tier,
270 required_count=required_count,
271 supplied_count=supplied,
272 posts=tuple(posts),
273 )
276#: Fields :func:`keel.artifacts.render_jury_verdict` accepts from a jury record.
277#: Named rather than splatted: the record comes from a parsed ai-jury report, and
278#: an unexpected key there must be dropped, never forwarded into a renderer.
279_JURY_FIELDS = (
280 "verdict",
281 "participants",
282 "participating_vendors",
283 "panelists",
284 "findings_summary",
285 "remaining_risks",
286)
289def _jury_post(
290 jury_record: dict[str, Any],
291 *,
292 head_sha: str | None,
293 pull_request: int,
294 run_id: str,
295) -> PostTarget:
296 if not isinstance(jury_record, dict):
297 raise ReviewError("jury record must be a JSON object")
298 fields = {key: jury_record[key] for key in _JURY_FIELDS if key in jury_record}
299 return PostTarget(
300 artifact="jury-verdict",
301 target_kind="pr",
302 target_number=pull_request,
303 run_id=jury_run_id(run_id),
304 marker=evidence.JURY_VERDICT_MARKER,
305 body=artifacts.render_jury_verdict(head_sha=head_sha, **fields),
306 )
309def _closure_posts(
310 closure_record: dict[str, Any],
311 *,
312 pull_request: int,
313 issue: int | None,
314 run_id: str,
315) -> list[PostTarget]:
316 if not isinstance(closure_record, dict):
317 raise ReviewError("closure file must contain a JSON object")
318 body = closure.render_closure_comment(closure_record)
319 sub_run_id = closure_run_id(run_id)
320 targets: list[PostTarget] = [
321 PostTarget(
322 artifact="closure-comment",
323 target_kind="pr",
324 target_number=pull_request,
325 run_id=sub_run_id,
326 marker=closure.COMMENT_MARKER,
327 body=body,
328 )
329 ]
330 if issue is not None:
331 targets.append(
332 PostTarget(
333 artifact="closure-comment",
334 target_kind="issue",
335 target_number=issue,
336 run_id=sub_run_id,
337 marker=closure.COMMENT_MARKER,
338 body=body,
339 )
340 )
341 return targets