ocr-post

  1#!/usr/bin/env python3
  2"""Post one Open Code Review run as a single advisory PR review (MIP-0060 §5.2 "Post").
  3
  4Advisory means: exactly one `COMMENT` review, never `REQUEST_CHANGES`, and exit 0 whatever the
  5JSON looks like — a broken review tool must not turn a PR red.
  6
  7Input schema read from alibaba/open-code-review at tag **v1.12.7**:
  8`internal/model/review.go` (`LlmComment`: path, content, suggestion_code, existing_code,
  9start_line, end_line, category, severity), `cmd/opencodereview/output.go` (`jsonOutput`:
 10status, llm{provider,model}, message, summary{files_reviewed,comments,elapsed,…}, comments,
 11warnings), and upstream's own consumer `scripts/github-actions/post-review-comments.js`, which
 12is where the posting rules come from: a finding is inline-able when start_line or end_line ≥ 1,
 13multi-line uses start_line + line on side RIGHT, and a ```suggestion block is emitted only when
 14suggestion_code *and* existing_code are both present.
 15
 16    ocr-post.py --json ocr.json --repo owner/name --pr 42 --head-sha "$SHA"
 17    ocr-post.py --json ocr.json --repo owner/name --pr 42 --head-sha x --diff d.patch --dry-run
 18    ocr-post.py --self-test
 19"""
 20
 21from __future__ import annotations
 22
 23import argparse
 24import json
 25import re
 26import subprocess
 27import sys
 28from pathlib import Path
 29
 30MARKER = "<!-- marola-ocr -->"
 31# MIP-0060 §8: low recall is by design, so silence must never read as a pass.
 32QUIET_NOT_APPROVAL = "A quiet run is **not** an approval — this reviewer's recall is low by design."
 33SEVERITY_RANK = {"critical": 4, "high": 3, "medium": 2, "low": 1}
 34MAX_BODY = 1200
 35MAX_CODE = 800
 36HUNK = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@")
 37MENTION = re.compile(r"(?<![\w/`])@([A-Za-z0-9][-A-Za-z0-9]{0,38})")
 38FIXTURES = Path(__file__).resolve().parent / "fixtures" / "ocr"
 39
 40
 41class GhError(RuntimeError):
 42    def __init__(self, status: int, message: str):
 43        super().__init__(message)
 44        self.status = status
 45
 46
 47def gh_api(method: str, path: str, payload=None, accept: str | None = None):
 48    """Every GitHub read and write goes through here, so --self-test can swap in a fake."""
 49    cmd = ["gh", "api", "-X", method, path]
 50    if accept:
 51        cmd += ["-H", f"Accept: {accept}"]
 52    if payload is not None:
 53        cmd += ["--input", "-"]
 54    p = subprocess.run(
 55        cmd,
 56        input=json.dumps(payload) if payload is not None else None,
 57        capture_output=True,
 58        text=True,
 59        check=False,
 60    )
 61    if p.returncode != 0:
 62        m = re.search(r"HTTP (\d{3})", p.stderr)
 63        raise GhError(int(m.group(1)) if m else 0, (p.stderr or "gh api failed").strip())
 64    if accept and "json" not in accept:
 65        return p.stdout
 66    return json.loads(p.stdout) if p.stdout.strip() else None
 67
 68
 69# --- the OCR JSON -----------------------------------------------------------------
 70
 71
 72def _int(v) -> int:
 73    return v if isinstance(v, int) and not isinstance(v, bool) else 0
 74
 75
 76def normalise(c: dict) -> dict:
 77    return {
 78        "path": str(c.get("path") or "").strip().lstrip("./"),
 79        "content": str(c.get("content") or ""),
 80        "start_line": _int(c.get("start_line")),
 81        "end_line": _int(c.get("end_line")),
 82        "severity": str(c.get("severity") or "").strip().lower(),
 83        "category": str(c.get("category") or "").strip().lower(),
 84        "suggestion_code": str(c.get("suggestion_code") or ""),
 85        "existing_code": str(c.get("existing_code") or ""),
 86    }
 87
 88
 89def load_findings(path: str) -> tuple[dict, list[dict], str]:
 90    """(document, findings, reason). A non-empty reason means: degrade to summary-only."""
 91    try:
 92        raw = Path(path).read_text(encoding="utf-8")
 93    except OSError as e:
 94        return {}, [], f"cannot read {path} ({e.strerror or e})"
 95    if not raw.strip():
 96        return {}, [], f"{Path(path).name} is empty — ocr wrote no output"
 97    try:
 98        doc = json.loads(raw)
 99    except json.JSONDecodeError as e:
100        return {}, [], f"{Path(path).name} is not valid JSON ({e.msg}, line {e.lineno})"
101    if not isinstance(doc, dict):
102        return {}, [], "top-level JSON is not an object — unknown output shape"
103    if doc.get("comments") is None and doc.get("status") == "failed":
104        # A failed run carries `comments: null`, not `[]` (seen in a real v1.12.7 run, fixture
105        # failed-run.json); the per-file reasons are in manifest.coverage.failed.
106        failed = ((doc.get("manifest") or {}).get("coverage") or {}).get("failed") or []
107        why = next(
108            (str(f.get("reason")) for f in failed if isinstance(f, dict) and f.get("reason")), ""
109        )
110        return doc, [], f"ocr failed on {len(failed)} file(s)" + (f" — {why}" if why else "")
111    if not isinstance(doc.get("comments"), list):
112        return doc, [], "no `comments` array in the output — unknown output shape"
113    findings = [normalise(c) for c in doc["comments"] if isinstance(c, dict)]
114    return doc, [f for f in findings if f["path"]], ""
115
116
117# --- the PR diff ------------------------------------------------------------------
118
119
120def right_lines(diff_text: str) -> dict[str, set[int]]:
121    """Line numbers addressable on the RIGHT side of a unified diff, per path."""
122    out: dict[str, set[int]] = {}
123    path, new_no, new_left, old_left = None, 0, 0, 0
124    for line in diff_text.splitlines():
125        if line.startswith("+++ "):
126            p = line[4:].split("\t")[0].strip()
127            path = None if p == "/dev/null" else (p[2:] if p.startswith("b/") else p)
128            new_left = old_left = 0
129            continue
130        m = HUNK.match(line)
131        if m:
132            new_no = int(m.group(3))
133            new_left = int(m.group(4) or 1)
134            old_left = int(m.group(2) or 1)
135            continue
136        if path is None or (new_left <= 0 and old_left <= 0):
137            continue
138        if line.startswith("\\"):
139            continue
140        if line.startswith("+"):
141            out.setdefault(path, set()).add(new_no)
142            new_no += 1
143            new_left -= 1
144        elif line.startswith("-"):
145            old_left -= 1
146        elif line.startswith(" ") or line == "":
147            out.setdefault(path, set()).add(new_no)
148            new_no += 1
149            new_left -= 1
150            old_left -= 1
151        else:
152            new_left = old_left = 0
153    return out
154
155
156def inline_position(f: dict, lines: dict[str, set[int]]) -> dict | None:
157    known = lines.get(f["path"])
158    if not known:
159        return None
160    s, e = f["start_line"], f["end_line"]
161    if 1 <= s < e and s in known and e in known:
162        return {"line": e, "start_line": s, "side": "RIGHT", "start_side": "RIGHT"}
163    one = e if e >= 1 else s
164    return {"line": one, "side": "RIGHT"} if one >= 1 and one in known else None
165
166
167# --- text the model wrote: never trusted -------------------------------------------
168
169
170def sanitise(text: str, limit: int = MAX_BODY) -> str:
171    """Strip HTML comments (a finding must not forge the sticky marker) and defuse @mentions."""
172    t = re.sub(r"<!--.*?-->", "", text, flags=re.S)
173    t = t.replace("<!--", "&lt;!--").replace("-->", "--&gt;")
174    t = MENTION.sub(r"`@\1`", t)
175    if len(t) > limit:
176        t = t[: limit - 16].rstrip() + "\n\n…(truncated)"
177    return t.strip()
178
179
180def fence_for(code: str) -> str:
181    longest = max((len(r) for r in re.findall(r"`+", code)), default=0)
182    return "`" * max(3, longest + 1)
183
184
185def label(f: dict) -> str:
186    tag = "/".join(x for x in (f["category"], f["severity"]) if x)
187    return f"[{tag}]" if tag else ""
188
189
190def comment_body(f: dict) -> str:
191    body = sanitise(f["content"]) or "_(the model wrote no text)_"
192    if label(f):
193        body = f"**{label(f)}** {body}"
194    # Upstream's own condition for treating the field as a code suggestion.
195    if f["suggestion_code"] and f["existing_code"]:
196        code = re.sub(r"<!--.*?-->", "", f["suggestion_code"], flags=re.S)[:MAX_CODE]
197        fence = fence_for(code)
198        body += f"\n\n{fence}suggestion\n{code.rstrip()}\n{fence}"
199    return body
200
201
202def one_line(f: dict, n: int = 140) -> str:
203    text = sanitise(f["content"], n + 40).replace("\n", " ").strip()
204    return (text[: n - 1] + "…") if len(text) > n else text
205
206
207# --- the two things we post --------------------------------------------------------
208
209
210def clean_field(v: str, n: int = 60) -> str:
211    return re.sub(r"[`|\r\n]", "", str(v)).strip()[:n] or "unknown"
212
213
214def header(doc: dict, args, n_findings: int) -> str:
215    s = doc.get("summary") if isinstance(doc.get("summary"), dict) else {}
216    llm = doc.get("llm") if isinstance(doc.get("llm"), dict) else {}
217    model = clean_field(args.model or llm.get("model") or "unknown")
218    provider = clean_field(args.provider or llm.get("provider") or "local", 24)
219    files = _int(s.get("files_reviewed")) if isinstance(s.get("files_reviewed"), int) else "?"
220    elapsed = clean_field(args.elapsed or s.get("elapsed") or "?", 24)
221    return (
222        f"`marola-ocr` · advisory · {model} ({provider}) · ocr "
223        f"{clean_field(args.ocr_version or 'unknown', 24)} · "
224        f"{files} files, {n_findings} findings, {elapsed}"
225    )
226
227
228def summary_body(head: str, posted: int, dropped: list[tuple[dict, str]], notes: list[str]) -> str:
229    out = [MARKER, head, ""]
230    out.append(f"{posted} finding(s) posted inline on this PR's diff.")
231    if dropped:
232        out += ["", f"Not posted inline ({len(dropped)}):"]
233        for f, why in dropped:
234            where = f"{f['path']}:{f['end_line'] or f['start_line']}"
235            out.append(f"- `{where}` {label(f)} — {why} — {one_line(f)}".replace(" —  — ", " — "))
236    for n in notes:
237        out += ["", n]
238    out += ["", QUIET_NOT_APPROVAL, ""]
239    return "\n".join(out)
240
241
242def post_review(api, repo: str, pr: int, payload: dict) -> int:
243    """Returns how many inline comments landed; -1 if no review could be posted at all."""
244    try:
245        api("POST", f"repos/{repo}/pulls/{pr}/reviews", payload)
246        return len(payload.get("comments", []))
247    except GhError as e:
248        if e.status != 422:
249            print(f"ocr-post: review not posted ({e})", file=sys.stderr)
250            return -1
251        retry = {k: v for k, v in payload.items() if k != "comments"}
252        try:
253            api("POST", f"repos/{repo}/pulls/{pr}/reviews", retry)
254        except GhError as e2:
255            print(f"ocr-post: summary-only retry also failed ({e2})", file=sys.stderr)
256            return -1
257        return 0
258
259
260def upsert_sticky(api, repo: str, pr: int, body: str) -> str:
261    for page in range(1, 6):
262        items = api("GET", f"repos/{repo}/issues/{pr}/comments?per_page=100&page={page}") or []
263        for c in items:
264            if MARKER in (c.get("body") or ""):
265                api("PATCH", f"repos/{repo}/issues/comments/{c['id']}", {"body": body})
266                return "updated"
267        if len(items) < 100:
268            break
269    api("POST", f"repos/{repo}/issues/{pr}/comments", {"body": body})
270    return "created"
271
272
273# --- wiring ------------------------------------------------------------------------
274
275
276def run(args, api) -> int:
277    doc, findings, reason = load_findings(args.json)
278    if reason:
279        head = header(doc, args, 0)
280        body = f"{MARKER}\n{head}\n\nno review this run: {reason}\n\n{QUIET_NOT_APPROVAL}\n"
281        if args.dry_run:
282            print("(no review posted)")
283            print(body)
284        else:
285            upsert_sticky(api, args.repo, args.pr, body)
286        print(f"ocr-post: no review this run: {reason}", file=sys.stderr)
287        return 0
288
289    if args.diff:
290        diff = Path(args.diff).read_text(encoding="utf-8")
291    else:
292        diff = api(
293            "GET",
294            f"repos/{args.repo}/pulls/{args.pr}",
295            accept="application/vnd.github.v3.diff",
296        )
297    lines = right_lines(diff or "")
298
299    ordered = sorted(findings, key=lambda f: -SEVERITY_RANK.get(f["severity"], 0))
300    placed, dropped = [], []
301    for f in ordered:
302        pos = inline_position(f, lines)
303        if pos:
304            placed.append((f, pos))
305        else:
306            dropped.append((f, "outside this PR's diff"))
307    over = placed[args.max_inline :]
308    placed = placed[: args.max_inline]
309    dropped += [(f, f"over the inline cap of {args.max_inline}") for f, _ in over]
310
311    head = header(doc, args, len(findings))
312    review = {
313        "commit_id": args.head_sha,
314        "event": "COMMENT",
315        "body": f"{head}\n\nAdvisory only. {QUIET_NOT_APPROVAL}",
316        "comments": [{"path": f["path"], "body": comment_body(f), **pos} for f, pos in placed],
317    }
318
319    notes = []
320    if args.dry_run:
321        print(json.dumps(review, indent=2, ensure_ascii=False))
322        posted = len(placed)
323    else:
324        posted = post_review(api, args.repo, args.pr, review)
325        if posted < len(placed):
326            notes.append("The review carried no inline comments; every finding is listed here.")
327            dropped += [(f, "position rejected by GitHub") for f, _ in placed]
328            posted = max(posted, 0)
329
330    body = summary_body(head, posted, dropped, notes)
331    if args.dry_run:
332        print(body)
333    else:
334        print(f"ocr-post: {upsert_sticky(api, args.repo, args.pr, body)} the sticky summary")
335    return 0
336
337
338def build_parser() -> argparse.ArgumentParser:
339    ap = argparse.ArgumentParser(description=__doc__.split("\n")[0])
340    ap.add_argument("--json", help="ocr review --format json --output <this>")
341    ap.add_argument("--repo", help="OWNER/NAME")
342    ap.add_argument("--pr", type=int)
343    ap.add_argument("--head-sha")
344    ap.add_argument("--diff", help="unified diff file; default: fetch the PR's diff via gh")
345    ap.add_argument("--model", default="")
346    ap.add_argument("--provider", default="")
347    ap.add_argument("--ocr-version", default="")
348    ap.add_argument("--elapsed", default="")
349    ap.add_argument("--max-inline", type=int, default=15)
350    ap.add_argument("--dry-run", action="store_true")
351    ap.add_argument("--self-test", action="store_true")
352    return ap
353
354
355def main(argv=None) -> int:
356    args = build_parser().parse_args(argv)
357    if args.self_test:
358        return self_test()
359    missing = [n for n in ("json", "repo", "pr", "head_sha") if not getattr(args, n)]
360    if missing:
361        print(
362            f"ocr-post: missing --{', --'.join(m.replace('_', '-') for m in missing)}",
363            file=sys.stderr,
364        )
365        return 2
366    return run(args, gh_api)
367
368
369# --- self-test ---------------------------------------------------------------------
370
371
372class FakeGh:
373    """The whole GitHub side: a diff to serve, an issue-comment store, reviews received."""
374
375    def __init__(self, diff: str, reject_positions: bool = False):
376        self.diff, self.reject = diff, reject_positions
377        self.comments: list[dict] = []
378        self.reviews: list[dict] = []
379        self.patches = 0
380        self.next_id = 100
381
382    def __call__(self, method, path, payload=None, accept=None):
383        if accept and "diff" in accept:
384            return self.diff
385        if method == "POST" and path.endswith("/reviews"):
386            if self.reject and payload.get("comments"):
387                raise GhError(422, "HTTP 422: line must be part of the diff")
388            self.reviews.append(payload)
389            return {"id": 1}
390        if method == "GET" and "/issues/" in path:
391            return self.comments if "page=1" in path else []
392        if method == "PATCH" and "/issues/comments/" in path:
393            self.patches += 1
394            for c in self.comments:
395                if c["id"] == int(path.rsplit("/", 1)[1]):
396                    c["body"] = payload["body"]
397            return {"id": 1}
398        if method == "POST" and "/issues/" in path:
399            c = {"id": self.next_id, "body": payload["body"]}
400            self.next_id += 1
401            self.comments.append(c)
402            return c
403        raise AssertionError(f"unexpected {method} {path}")
404
405
406def self_test() -> int:  # noqa: C901 - a flat list of assertions reads better than helpers
407    fails = 0
408
409    def ok(cond, what):
410        nonlocal fails
411        if not cond:
412            fails += 1
413            print(f"  FAIL: {what}", file=sys.stderr)
414
415    diff = (FIXTURES / "diff.patch").read_text(encoding="utf-8")
416    lines = right_lines(diff)
417    ok(88 in lines["scripts/oods_ingest.py"], "an added line is addressable on the RIGHT side")
418    ok(200 not in lines["scripts/oods_ingest.py"], "a line outside every hunk is not")
419    ok(41 in lines["oods/src/main/scala/Planner.scala"], "the second file's hunk is parsed too")
420
421    def go(fixture, **kw):
422        argv = [
423            "--json", str(FIXTURES / fixture),
424            "--repo", "h0ffmann/marola", "--pr", "42", "--head-sha", "deadbee",
425            "--diff", str(FIXTURES / "diff.patch"), "--ocr-version", "1.12.7",
426        ]  # fmt: skip
427        for k, v in kw.items():
428            argv += [f"--{k}", str(v)]
429        api = FakeGh(diff)
430        return run(build_parser().parse_args(argv), api), api
431
432    rc, api = go("normal.json")
433    ok(rc == 0, "a normal run exits 0")
434    ok(len(api.reviews) == 1 and api.reviews[0]["event"] == "COMMENT", "one COMMENT review")
435    bodies = "\n".join(c["body"] for c in api.reviews[0]["comments"])
436    ok("`@hoffmann`" in bodies and "\n@hoffmann" not in bodies, "an @mention cannot ping anyone")
437    ok(MARKER not in bodies, "a finding cannot forge the sticky marker")
438    ok(len(api.comments) == 1 and MARKER in api.comments[0]["body"], "one sticky summary")
439    ok(QUIET_NOT_APPROVAL in api.comments[0]["body"], "the summary says a quiet run is no approval")
440
441    rc, api = go("out-of-diff.json")
442    ok(len(api.reviews[0]["comments"]) == 1, "the out-of-diff finding is not posted inline")
443    sticky = api.comments[0]["body"]
444    ok("Untouched.scala" in sticky and "outside this PR's diff" in sticky, "…it is counted instead")
445
446    rc, api = go("many.json")
447    posted = api.reviews[0]["comments"]
448    ok(len(posted) == 15, "inline comments are capped at 15 by default")
449    ranks = [
450        SEVERITY_RANK.get(b["body"].split("[")[1].split("/")[1].split("]")[0], 0) for b in posted
451    ]
452    ok(ranks == sorted(ranks, reverse=True), "the cap keeps the most severe findings, in order")
453    ok("over the inline cap of 15" in api.comments[0]["body"], "the rest are listed in the summary")
454    rc, api = go("many.json", **{"max-inline": 3})
455    ok(len(api.reviews[0]["comments"]) == 3, "--max-inline moves the cap")
456
457    api = FakeGh(diff)
458    argv = [
459        "--json", str(FIXTURES / "normal.json"), "--repo", "h0ffmann/marola", "--pr", "42",
460        "--head-sha", "deadbee", "--diff", str(FIXTURES / "diff.patch"),
461    ]  # fmt: skip
462    run(build_parser().parse_args(argv), api)
463    run(build_parser().parse_args(argv), api)
464    ok(len(api.comments) == 1 and api.patches == 1, "a second run PATCHes, never posts a twin")
465
466    for fixture, why in (
467        ("malformed.json", "not valid JSON"),
468        ("empty.json", "is empty"),
469        ("failed-run.json", "ocr failed on 4 file(s)"),
470    ):
471        rc, api = go(fixture)
472        ok(rc == 0, f"{fixture} still exits 0 — the check is advisory")
473        ok(not api.reviews, f"{fixture} posts no review")
474        ok(why in api.comments[0]["body"], f"{fixture} explains itself in the summary")
475        ok(
476            QUIET_NOT_APPROVAL in api.comments[0]["body"],
477            f"{fixture} still refuses to imply a pass",
478        )
479
480    api = FakeGh(diff, reject_positions=True)
481    run(build_parser().parse_args(argv), api)
482    ok(len(api.reviews) == 1 and "comments" not in api.reviews[0], "a 422 retries as summary-only")
483    ok("position rejected" in api.comments[0]["body"], "…and the summary keeps every finding")
484
485    print(f"ocr-post self-test: {'ok' if fails == 0 else f'{fails} FAILED'}")
486    return 0 if fails == 0 else 1
487
488
489if __name__ == "__main__":
490    sys.exit(main())